[.NET Core] Event source traces revision - Part 1 & 2

[cheenamalhotra]

The PR standardizes event source traces with additional info added where applicable like Client Connection Id, SPID, etc. as they're extremely helpful while debugging. I'll be working on all other classes (most importantly .NET Core) including Managed SNI later once this gets merged to avoid conflicts.

[JRahnama]

LGTM, it looks better now.

[Wraith2]

@cheenamalhotra is there a part 3? i'm working through a diff of some files and it's struck me that this PR has changed netcore but not netfx

[cheenamalhotra]

Yes, I'm working on Managed SNI and TDS Parser layers (PR on way..)
NetFx is low priority for these improvements, as we need these changes to debug and traces issues in Managed SNI first.

[Wraith2]

Ok. I'll put an item on my ever-expanding list of things to do.

I'm working to try and get as much shared or synced between netfx and netcore so these new changes stand out. Doing a single file at a time would leave the netfx tracing in a bad state so it'll need to be done in a single PR at a later date.

[cheenamalhotra]

Ok, thanks for keeping up with that! Please also keep your branches up-to-date and resolve conflicts in case you encounter (hopefully not many, since they're just trace changes).

At this point, our primary focus is on solving #659 and #422, and these improvements are helpful in investigations.
We will definitely make them uniform with NetFx in future 👍

[Wraith2]

I was going to have a look at #659 over the holiday but was diverted. My thought was to start in the mars connection demultiplexing code and make sure that the lookup was locked/concurrent safe because that was the only way I could think it would be possible to get a packet to the wrong virtual connection.

On #422 sadly the answer is that they shouldn't do what they're trying to do. It's a limitation of the way it works and getting around that required platform specific non-threadpool IO. I wrote up a big post on 13th December after I'd spent a few days on it. I'd be happy to talk that through with you if you think it'd help

As always, I'll try and avoid conflicts.

[cheenamalhotra]

That would be nice!

Please check my last comment on #659 maybe it rings a bell to you. it doesn't seem a concurrency issue, it's more due to uncleared information. To cut short, data from first connection is arriving in the "receivedPacketQueue" even after we closed the first connection (sent it to pool) since the physical connection is still active which is dequeued in second connection's query. There should be a cleanup/disposal happening before putting connection to pool (so we don't listen to more packets) or after connection is reactivated (cleanup everything) [I'm still trying to establish this part]. We do perform fResetConnection, so that's not related, because when RESET happens the queue already contains data on the client side.

We should be cleaning up MarsHandle or packets which are still connected to StateObj on the connection in pooling.
I also always hit the Debug assert in SNIPacket's destructor, and we have a lot of unreleased packets, maybe it's related?

For #422, I have some ideas to try, but we'll come to that after this more serious issue.