Skip to content
This repository has been archived by the owner on Nov 17, 2018. It is now read-only.

NullReferenceException in ActiveHandlerTrackingEntry with 2.2.0 preview 3 #194

Closed
martincostello opened this issue Oct 17, 2018 · 8 comments
Assignees
Milestone

Comments

@martincostello
Copy link
Contributor

martincostello commented Oct 17, 2018

I don't yet have a repro, but I've just updated an ASP.NET Core web app that uses IHttpClientFactory to 2.2.0 preview 3, and I've got some in-memory self-hosted tests failing because of the following exception causing dotnet test to terminate:

[18:18:48][dotnet test] The active test run was aborted. Reason: Unhandled Exception: System.NullReferenceException: Object reference not set to an instance of an object.
[18:18:48][dotnet test]    at Microsoft.Extensions.Http.ActiveHandlerTrackingEntry.Timer_Tick()
[18:18:48][dotnet test]    at Microsoft.Extensions.Http.ActiveHandlerTrackingEntry.<>c.<.cctor>b__22_0(Object s)
[18:18:48][dotnet test]    at System.Threading.TimerQueueTimer.CallCallback()
[18:18:48][dotnet test]    at System.Threading.TimerQueueTimer.Fire()
[18:18:48][dotnet test]    at System.Threading.TimerQueue.FireNextTimers()
[18:18:48][dotnet test]    at System.Threading.TimerQueue.AppDomainTimerCallback(Int32 id)

I guess that one of either _timer or _callback is somehow null:

private void Timer_Tick()
{
Debug.Assert(_callback != null);
Debug.Assert(_timer != null);
lock (_lock)
{
_timer.Dispose();
_timer = null;
_callback(this);
}
}

@martincostello
Copy link
Contributor Author

It's also not a one-off, as we run these tests in batches based on categories to get some parallelism across TeamCity jobs, and 4 different jobs (of 7) have all failed with the same exception.

@rynowak
Copy link
Member

rynowak commented Oct 17, 2018

Ah yes. It's probably the case that we need to null check the timer when we access it. This was a 2.2.0 change.

@rynowak rynowak added this to the 2.2.0 milestone Oct 17, 2018
@rynowak rynowak self-assigned this Oct 17, 2018
@palmtech
Copy link

Is there any workaround for this? All of our preview 3 containers terminate every few minutes with this error.

@martincostello
Copy link
Contributor Author

Looking at the code, I think the only workaround is to make the messsge handler lifetimes either Infinite or really long (~the usual lifetime of your containers).

rynowak added a commit that referenced this issue Oct 19, 2018
This issue here is that we were allowing multiple timers to start
because we never set `_timerInitialized`. This is a regression during
the milestone from introducing NonCapturingTimer.
rynowak added a commit that referenced this issue Oct 19, 2018
This issue here is that we were allowing multiple timers to start
because we never set `_timerInitialized`. This is a regression during
the milestone from introducing NonCapturingTimer.
@rynowak
Copy link
Member

rynowak commented Oct 19, 2018

Fixed this for 2.2 - sorry I don't think there's a workaround here. This is just broken. What happens is that an additional timer is started each time you request an HTTP client within the lifetime of a single handler.

@palmtech
Copy link

Ok, might be worth adding it to the known issues. Preview 3 is probably a skip for most microservice-ish apps.

@rynowak
Copy link
Member

rynowak commented Oct 20, 2018

Yeah that's a fair point, will do.

@rynowak
Copy link
Member

rynowak commented Oct 20, 2018

Added aspnet/Announcements#323

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants