System.NullReferenceException: Object reference not set to an instance of an object at .Kestrel.Infrastructure.Headers.Initialize

[mnltejaswini]

I am running the following exception when running MusicStore app

warn: Microsoft.AspNetCore.Server.Kestrel[0] Connection processing ended abnormally warn: Microsoft.AspNetCore.Server.Kestrel[0] Connection processing ended abnormally warn: Microsoft.AspNetCore.Server.Kestrel[0] Connection processing ended abnormally System.NullReferenceException: Object reference not set to an instance of an object. at Microsoft.AspNetCore.Server.Kestrel.Infrastructure.Headers.Initialize(DateHeaderValueManager dateValueManager) at Microsoft.AspNetCore.Server.Kestrel.Http.Frame1.d__2.MoveNext()
System.NullReferenceException: Object reference not set to an instance of an object.
at Microsoft.AspNetCore.Server.Kestrel.Infrastructure.Headers.Initialize(DateHeaderValueManager dateValueManager)
at Microsoft.AspNetCore.Server.Kestrel.Http.Frame1.<RequestProcessingAsync>d__2.MoveNext() System.NullReferenceException: Object reference not set to an instance of an object. at Microsoft.AspNetCore.Server.Kestrel.Infrastructure.Headers.Initialize(DateHeaderValueManager dateValueManager) at Microsoft.AspNetCore.Server.Kestrel.Http.Frame1.d__2.MoveNext()`

[rynowak]

We hit this today doing some perf testing with MusicStore. Seems to happen for the first few requests and then never again

[benaadams]

Start up race that seemed to work prior to #810 by using an null array for the date header value which FrameResponseHeaders.CopyToFast would skip therefore not outputting a date header.

With the date bytes and date string now in a object; that object is null - which causes the null reference when resolving the bytes at start-up.

[halter73]

@benaadams This is a new race that didn't exists before #810.

First of all, _dateBytes0 and _dateBytes1 were never null, so CopyToFast doesn't influence this. Secondly, unlike now, the date byte array and string returned by GetDateHeaderValue and GetDateHeaderValueBytes were always initialized before they were returned.

Prior to #810, during timer startup, GetDateHeaderValue and GetDateHeaderValueBytes both would always called PrepareDateValues. If the timer hadn't started (as determined by_timerIsRunning) , PrepareDateValues would call StartTimer which would only set _timerIsRunning = true after calling SetDateValues(now);.

Since #810 this logic is now flipped. Now instead of using _timerIsRunning, GetDateHeaderValues uses _hadRequestsSinceLastTimerTick to determine whether or not to call StartTimer in conjunction with checking _timerIsRunning in PrepareDateValues itself. Now this causes a race when first starting the timer because _hadRequestsSinceLastTimerTick is set to true before _dateValues is initialized which allows GetDateHeaderValues to return null on calls made quickly enough after the first one.

I think the fix is pretty simple: just go back to to always calling StartTimer iff _timerIsRunning is false. Checking whether _hadRequestsSinceLastTimerTickis false in GetDateHeaderValues and only calling PrepareDateValues then makes no sense to me anyway. Previously _hadRequestsSinceLastTimerTick was only read to determine whether or not to update _lastRequestSeenTicks.

[rynowak]

/cc @sebastienros FYI

[sebastienros]

I approve this fix, good job guys. Let's fix all the race issues one at a time.