-
Notifications
You must be signed in to change notification settings - Fork 770
4.x: Fix timed delay hangs with dotCover and the DefaultScheduler #579
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
{ | ||
_action = Stubs<object>.Ignore; | ||
_timer = TimerStubs.Never; | ||
_state = null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see a race here where _state could be set null but the timer will execute, passing the wrong state (it doesn't help that _action is set to Stubs.Ignore before, that might be rearranged). Maybe timer._action should only be called when TryDispose was true, setting _action and _state by itself (or by some DisposeCore method)?
Do you know anything about the issue or bug here? Seems that the SpinWait would loop forever? |
I don't know, I can't switch to debug while running with coverage enabled. Maybe there is a way to trigger a thread dump externally? |
I could give it a try. |
Do you know what process will eventually host our code? Everything I debug doesn't run anything form System.Reactive. |
Either a |
I can't reproduce the hanging tests under coverage. |
Were you running dotCover or microsoft's own coverage tool of VS Enterprise? |
DotCover within VS |
There could be a probabilistic signal loss from the timer. You could try and add a loop around the body of [Fact]
public void Delay_TimeSpan_DefaultScheduler()
{
for (int i = 0; i < 10000; i++)
Assert.True(Observable.Return(1).Delay(TimeSpan.FromMilliseconds(1))
.ToEnumerable().SequenceEqual(new[] { 1 }));
} |
Can't really reproduce it but the fix is straightforward and the SpinWait-solution is wacky enough that this should be merged. |
For some reason, dotCover would hang in various timed test which use the
DefaultScheduler
. I don't know why, but removing that emptytry-catch
along with the atomic dispose management solves the issue.In addition, the
Virtual_ThreadSafety()
was running extremely slow in its original form (more than 5 minutes). The reduced time amount and a more close thread-synchronization should be faster now while preserving the behavior under test.