17

I have a strange problem in my MVC 4.0 application. I use REST web services (Amazon Associate) . I created a method, which I use from everywhere. The shortened version is this:

    private async Task<XElement> GetRequest(string url)
    {
        string myresponse;
        HttpResponseMessage response = null;
        HttpClient client = new HttpClient();            
        try
        {
            response = await client.GetAsync(url);
            myresponse = response.Content.ToString();
            if (myresponse.Contains("503"))
            {
                myTrace.WriteLine("503 Sleep.....");
                Thread.Sleep(3000); // looks like amazon us does not like fast requests....
                return await GetRequest(url); //restart after pausing....
            }
        }
        catch (TaskCanceledException ex)
        {
            myTrace.WriteLine("TaskCancelled From GetRequest: " + ex);
            return null;
        }

        catch (HttpRequestException ex)
        {
            myTrace.WriteLine("RequestException Sleep.....");
            Thread.Sleep(300000); // 5 minutes de pause 
        }

        catch (Exception ex)
        {
            myTrace.WriteLine("From GetRequest: " + ex);
            return null;
        }

        try
        {
            XElement content = await response.Content.ReadAsAsync<XElement>();
            response.Dispose();
            client.Dispose();
            return content;
        }
        catch (Exception)
        {
            return null;
        }
    }

Nothing fancy, it does work perfectly well....But, now, on a specific call, it bombs on client.GetAsync(url). At first I suspected something in the url to be wrong, so I grabbed it from a debugger session and pasted it directly in my browser, got the expected answer...

So, nothing wrong with the URL. Made a little Unit Test, works just fine with that same specific URL...

As it bombs in the debugger, difficult to see what's wrong. (There are no exceptions thrown!). Finally, I saw with IntelliTrace that there ARE exceptions, seemingly inside System.Threading.Tasks. Difficult to pin point, as the call Stack is a bit confusing for my NON expert eyes....

Here is the call stack I get from a previous pass in the code:

>   System.Web.dll!System.Web.ThreadContext.AssociateWithCurrentThread(bool setImpersonationContext = {unknown})    C#
System.Web.dll!System.Web.HttpApplication.OnThreadEnterPrivate(bool setImpersonationContext = {unknown})    C#
System.Web.dll!System.Web.HttpApplication.OnThreadEnter()   C#
System.Web.dll!System.Web.HttpApplication.System.Web.Util.ISyncContext.Enter()  C#
System.Web.dll!System.Web.Util.SynchronizationHelper.SafeWrapCallback(System.Action action = {unknown}) C#
System.Web.dll!<>c__DisplayClass9.AnonymousMethod(System.Threading.Tasks.Task _ = {unknown})    C#
mscorlib.dll!System.Threading.Tasks.ContinuationTaskFromTask.InnerInvoke()  C#
mscorlib.dll!System.Threading.Tasks.Task.Execute()  C#
mscorlib.dll!System.Threading.Tasks.Task.ExecutionContextCallback(object obj = {unknown})   C#
mscorlib.dll!System.Threading.ExecutionContext.RunInternal(System.Threading.ExecutionContext executionContext = {unknown}, System.Threading.ContextCallback callback = {unknown}, object state = {unknown}, bool preserveSyncCtx = {unknown})   C#
mscorlib.dll!System.Threading.ExecutionContext.Run(System.Threading.ExecutionContext executionContext = {unknown}, System.Threading.ContextCallback callback = {unknown}, object state = {unknown}, bool preserveSyncCtx = {unknown})   C#
mscorlib.dll!System.Threading.Tasks.Task.ExecuteWithThreadLocal(ref System.Threading.Tasks.Task currentTaskSlot = {unknown})    C#
mscorlib.dll!System.Threading.Tasks.Task.ExecuteEntry(bool bPreventDoubleExecution = {unknown}) C#
mscorlib.dll!System.Threading.Tasks.Task.System.Threading.IThreadPoolWorkItem.ExecuteWorkItem() C#
mscorlib.dll!System.Threading.ThreadPoolWorkQueue.Dispatch()    C#
mscorlib.dll!System.Threading._ThreadPoolWaitCallback.PerformWaitCallback() C#

Anyway, this looks definitely linked to Tasks, Async, Background workers, etc... Is there a good way to "clear" all other running tasks, to avoid this problem?

Thanks for your help, Bernard.

BernardG
  • 1,956
  • 3
  • 17
  • 25
  • First thing to do: change your exception handling. You're currently losing information by just returning null if anything goes wrong, with no logging or anything to give you more diagnostic information. Also note that you're never disposing the response. – Jon Skeet Apr 17 '13 at 09:11
  • Additionally, you say "there are no exceptions thrown" and yet your question title talks about a NullReferenceException. Which is it? – Jon Skeet Apr 17 '13 at 09:12
  • Thanks for your quick answer, John. as I said, this is a shortened version, the 'real' one does handle exceptions properly (Or so do I believe...). Now, about exceptions thrown, the proper answer is "both"...No exceptions are thrown by the code, BUT, after a second pass in that same code with the debugger, IntelliTrace show me that there was a "silent" exception in the previous pass. is that clearer? – BernardG Apr 17 '13 at 09:28
  • Not really, to be honest. I don't know what you mean by a "silent" exception, we don't have a stack trace for the NullReferenceException that the title talks about, and we don't know what you mean by "it bombs" if no exception escapes your method. – Jon Skeet Apr 17 '13 at 09:30
  • Just edited my post to put the 'real' method, and the call stack I get with IntelliTrace help. – BernardG Apr 17 '13 at 09:55
  • What does that call stack show though? Is that an exception? It's still not clear where NullreferenceException comes in, and you still haven't given any indication of what you mean by "it bombs". – Jon Skeet Apr 17 '13 at 09:57
  • Also note that using Thread.Sleep in an async method is a really bad idea - that will block a method which isn't meant to block. Use `await Task.Delay` instead, to "sleep asynchronously". And also, you don't log the exception in the `HttpRequestException` case - why not? – Jon Skeet Apr 17 '13 at 09:58
  • And finally (for the moment) you're only disposing of the client and the response in the success case. That's a bad idea. Just use a `using` statement as normally, so that it will *always* be disposed. – Jon Skeet Apr 17 '13 at 09:59
  • Thanks for your help, John, I will do as you say. What I means by it bombs is that it exit the debugger sessions without any kind of error or exception. When the marker is on the line "response = await client.GetAsync(url)" and click on "Step Into", it just ends debugging.. – BernardG Apr 17 '13 at 10:05
  • "you don't log the exception in the HttpRequestException case - why not? " When I saw this exception, I was hitting too quickly on the servers, and they throttled my IP, so I just needed to wait. But yes, maybe I should.. – BernardG Apr 17 '13 at 10:08
  • And what happens when you're *not* debugging? You say you're unable to reproduce this in a unit test - is there any way of reproducing it in (say) a simple console app? – Jon Skeet Apr 17 '13 at 10:09
  • When I am not debugging, I just get a null back, silently (no exceptions, no errors) BUT, this same method has already been called twice without any problem... I could try from a console app, but I am not sure why it would be different from a test, as it looks like the problem arise only after a serie of calls. – BernardG Apr 17 '13 at 10:14
  • So make the console app make a series of calls... do anything to try to reproduce this in a way that ideally *we* can execute as well. – Jon Skeet Apr 17 '13 at 10:15
  • OK, will do. I'll be back here once I have been able to do this. Thanks again! – BernardG Apr 17 '13 at 10:19
  • Well, I get the explanation. It had nothing to do with GetRequest, but with the way it was called. In that particular case, at the root of a cascade of calls, I have a case switch, which was calling SYNCHRONEOUSLY instead of Async. No complains nowhere, everything was seen as fine by the compiler, BUT for those exceptions inside system.threading.tasks... I will clean my code following your recommendations, John, and lesson learned. If you use async, it has to be used the whole nine yards, or nowhere.... – BernardG Apr 17 '13 at 10:39
  • @BernardG Yeah, if you `Wait()` on an async method, you will get a deadlock in most cases. – svick Apr 17 '13 at 11:53
  • 1
    I'm getting a similar error and stacktrace, but with no indication as to where the exception is being thrown. Does anyone have any suggestions as to how to locate the offending line(s) of code? – David Keaveny Jun 20 '19 at 12:22

3 Answers3

33

Adding on to @kcar's answer, I had a very similar issue where there were multiple awaits on a code path that had a single method that was not awaited, like:

public async Task JsonResult BookThing(InputModel model)
{
    // Do some stuff
    thisIsAnAsyncMethod(Model model); // Fire and forget
    return Json(null);
}

protected async Task thisIsAnAsyncMethod(Model model)
{
    await oneThing();
    await anotherThing();
    await somethingElse();
}

That caused awaits to randomly fail without letting me catch the Exception - because TPL was trying to rejoin a Context that had been nulled out, so it was throwing a NullReferenceException outside of try/catch.

This is very hard to diagnose. In Production you won't see anything in try/catch, and in Visual Studio which await gets scheduled to rejoin the original context is somewhat random - it depends on what the TaskScheduler happens to decide to do.

If you didn't want to fire and forget the obvious answer is to await the async method - you'll have a compiler warning reminding you to do so.

If you did want to fire and forget the solution is to explicitly start a new Task. The best way to do that is covered in this answer about fire and forget Tasks.

Community
  • 1
  • 1
Chris Moschini
  • 36,764
  • 19
  • 160
  • 190
  • 1
    I don't think I've ever used the search box in the Visual Studio error list to search for `await` before, but if you have lots of unfixed compiler warnings (cough cough) then just search for await to find them all :) – Simon_Weaver Sep 12 '18 at 02:35
7

From the looks of it, your code can complete before one of the threads put to sleep in an exception event from the first try block are resolved, so after 5 minutes when they wake up, there isn't an original thread to rejoin resulting in a NullReferenceException from AssociateWithCurrentThread

kcar
  • 833
  • 9
  • 15
0

When I was getting the exception and call stack shown above, it was because I was trying to do a "fire and forget" execution using async, which was a really bad idea. I switched to spinning of a new thread for what I wanted and the crashes went away.

Bryan Legend
  • 6,790
  • 1
  • 59
  • 60