Merge 5.0.2

[fredericDelaporte]

No description provided.

[hazzik]

Ooops.

[fredericDelaporte]

Ok, does not look as what it should be. Going to restore the branch.

[fredericDelaporte]

You done it before me? I got an "Everything up-to-date" at time of doing it.

[hazzik]

Yes, sorry

[fredericDelaporte]

Not a trouble, so closing this PR, going to do another one from a branch on my repo.

[hazzik]

Just wonder what shall the constructor of SessionIdLoggingContext look like? I got this:

public SessionIdLoggingContext(Guid id)
{
	if (id == Guid.Empty) return;
	_oldSessonId = SessionId;
	if (id == _oldSessonId) return;
	_hasChanged = true;
	SessionId = id;
}

[hazzik]

@fredericDelaporte don't worry, I'll merge manually.

[fredericDelaporte]

Yes, my bad commit would have shown you how I have fixed it, but now it is no more!

It was:

public SessionIdLoggingContext(Guid id)
{
	var trackedId = id == Guid.Empty ? default(Guid?) : id;
	_oldSessonId = SessionId;
	if (trackedId == _oldSessonId) return;
	_hasChanged = true;
	SessionId = trackedId;
}

It handles "mixed case", where some session would have tracking enabled while other would not.

[fredericDelaporte]

@fredericDelaporte don't worry, I'll merge manually.

As you want. As the TC release build was ended I have switched to finishing the publishing (done now), and I was about to do the merge.

[hazzik]

Your version of the constructor does read the session from the context when tracking is disabled.

[fredericDelaporte]

It just reads whether the lazy is initialized when no sessions are tracking.
If one is tracking, anyway all sessions logging will start to read the context since they directly call the context property, which will find the lazy initialized.

So reading it on "context set" is then just a very small overhead compared to all the reads that will anyway occurs on each log call. And it avoids having non tracking session logging the session id of a tracking one.

[hazzik]

But lazy isn't used in Full .NET Fx

[fredericDelaporte]

You are right. But the reasoning still holds true: CallContext is anyway read at each logs done by any session, be it tracking or not. Sparring a read when the session sets its context is unlikely to be significant. And sparring it creates a bug (non-tracking session may log session id of tracking one).

For checking all that I then need to do a merged branch and run the bench on it.

[hazzik]

CallContext is anyway read at each logs done by any session, be it tracking or not.

As I understand this happens only if you run NHProf, which messes with the SessionIdLoggingContext.SessionId. Otherwise, no one reads it. I don't really worry about that case.

And sparring it creates a bug (non-tracking session may log session id of tracking one).

As I understand this could happen only if 2 sessions from different session factories run alongside in the same thread. (Or someone messes with the options before creating the session).

UPD: The "untracking session" should start running while the "tracking session" is running an operation in the same thread. I don't think that this is possible at all.

[fredericDelaporte]

UPD: The "untracking session" should start running while the "tracking session" is running an operation in the same thread. I don't think that this is possible at all.

No, just use two session factories on the same thread, one tracking, the other one not tracking. Open a session from the first with an interceptor using a session from the second. (Quite like the "interlaced" test I have added in #1455.)

Yeah that is quite contrived, that is why your merge choice is ok for me anyway.

(Upd: too used of clicking that "delete" button below, I fear I will end-up clicking it if I keep going on that PR.)

[fredericDelaporte]

@hazzik I fear staging the fix to default invalid merge has been missed, master is broken. I am going to commit the fix. (Done: ef86fdf)

[hazzik]

Thanks @fredericDelaporte

[hazzik]

I’ve protected the 5.0.x branch, so, no more delete button.

[hazzik]

@fredericDelaporte the tests are still failing. Strangely enough, they don't fail when they run in isolation.

[hazzik]

@fredericDelaporte ProcessHelper is broken. The test interfering with #1391 is ReconnectAfterClose. This test is trying to start a process on an already disposed session which obviously fails. But before it fails it is able to create a new SessionIdLoggingContext. Which later will not be disposed.

Addressed with 63dee05.