Access to disposed closure warning using Azure Transient Fault Handling Retry Policy

All, I have posted this question to StackOverflow, but I wanted to post here as well to hopefully get a more targeted community.

We have a worker role which processes records and sends Azure service bus messages as needed based on the results of the query, this is basically a queue processing service. As part of the best practices of using SQL Azure, we have wrapped all of our query statements with a retry policy (this detects transient errors and will retry based on the defined policy). Note that we actually send the message from within the using statement so there is no 'leak' of the db variable.

Inside of our using statement, ReSharper is throwing up the 'Access to Disposed Closure' warning, most likely because we are passing our DataContext as a func parameter of the retry policy.  The retry policy will not continue in the code block until the retry either succeeds or fails, so I don't believe that there is any potential that the db variable would be referenced after the using statement disposes the db variable.

My question is, am I OK in my assumption that ReSharper is not detecting this pattern correctly or are there alternative methods in how we write these functions in order to prevent the warning above?  Note, this code runs fine in our environment and has not caused any issues, I suppose I am just wanting clarification/validation on my assumptions.

The Code
The db variable in the retryPolicy.ExecuteAction is what is getting flagged (i have underlined it to help show the exact location)

using (var db = new MyEntities())
{
   var thingsToUpdate = retryPolicy.ExecuteAction(() => db.QueueTable.Where(x => x.UpdateType == "UpdateType" && x.DueNext < DateTime.UtcNow).Take(30).ToList());
   if (!thingsToUpdate.Any())
   {
      return;
   }
   while (thingsToUpdate.Any())
   {
      var message = new ServiceMessage{
                            Type = "UpdateType",
                            Requests = thingsToUpdate.Select(x => new ServiceMessageRequest
                                {
                                    LastRan = x.LastRan,
                                    ParentItemId = x.ThingId,
                                    OwnerId = x.Thing.ForiegnKeyid
                                }).ToList()
                        };
      SendMessage("UpdateType", message);
      foreach (var thing in thingsToUpdate )
      {
          thing.LastRan = DateTime.UtcNow;
          thing.DueNext = DateTime.UtcNow.AddMinutes(10);
      }
      retryPolicy.ExecuteAction(() => db.SaveChanges());
      thingsToUpdate = retryPolicy.ExecuteAction(() => db.QueueTable.Where(x => x.UpdateType == "UpdateType" && x.DueNext < DateTime.UtcNow).Take(30).ToList()));
     }
}
3 comments
Comment actions Permalink

ReSharper sees that you're using this variable in the lambdas passed to ExecuteAction, but doesn't know what the ExecuteAction function does with the lambda. If it holds onto the lambda (stores it in a field, perhaps), then tries to use it later, the db variable will be disposed, and your code will have unexpected side effects (most likely exceptions). This is what ReSharper is trying to protect you from.

However, since ReSharper doesn't know what ExecuteAction does with the lambda, it's also possible that your code is just fine. If ExecuteAction executes the lambda immediately, then the scope of the variable being disposed doesn't matter. In this case, ReSharper is warning you unnecessarily, and it's perfectly safe to ignore the warning.

(And indeed, this is why the highlight is a warning and not an error - ReSharper doesn't know if you're doing something *actually* wrong, or just *potentially* wrong, so it warns you.)

You have 3 choices:

1) Pay no attention to the warning
2) Disable the warning with comments. Use the quick-fix alt-enter to create the comments, and just surround the whole method. This turns off ReSharper's analysis for "access to disposed closure" in between the comments
3) If you control the assembly, you can add an attribute to the ExecuteAction method to tell ReSharper that the lambda is executed immediately. Once ReSharper knows that, the warning will disappear, because it knows you'll only access "db" *before* it's disposed.

If you don't have source access to the assembly, you can add the attribute via an annotations file, which can either sit in the same folder as the assembly, or in the Program Files\JetBrains\ReSharper\v7.1\bin\ExternalAnnotations folder. Here's an example for a similar ReSharper warning: http://hmemcpy.com/2013/02/preventing-resharpers-implicitly-captured-closure-warning-in-fakeiteasy-unit-tests/

Thanks
Matt

0
Comment actions Permalink

Matt,

Thanks for the reply and the clarification on why the warning message is being presented.  Indeed the ExecuteAction method does return immediately and does not hold onto the db variable for later use, so it sounds like I will be OK.  I will most likely suppress the warnings with the inline comments as this is the microsoft enterprise library assemblies...and I don't have control of those.   

0
Comment actions Permalink

Hi Tommy,

I wrote that blog post Matt was referring to. I wrote another one, further expanding on the cool things possible via external annotations: http://hmemcpy.com/2013/03/even-more-resharper-annotations-hackery-marking-3rd-party-code-as-obsolete/

You don't need to have the source code to apply external annotation - it's all done via external XML files. While ReSharper comes with many built-in annotations for other Microsoft.Practices.* assemblies (mainly, Prism), some things are missing.
Have a look at [ReSharper Install Dir]\Bin\ExternalAnnotations\Microsoft - you could add a folder here called Microsoft.Practices.TransientFaultHandling.Core, and create an xml file called Attributes.xml with the following content:

<?xml version="1.0" encoding="utf-8"?> <assembly name="Microsoft.Practices.TransientFaultHandling.Core">   <member name="M:Microsoft.Practices.TransientFaultHandling.RetryPolicy.ExecuteAction``1(System.Func{``0})">     <parameter name="func">       <attribute ctor="M:JetBrains.Annotations.InstantHandleAttribute.#ctor" />     </parameter>   </member>   <member name="M:Microsoft.Practices.TransientFaultHandling.RetryPolicy.ExecuteAction(System.Action)">     <parameter name="action">       <attribute ctor="M:JetBrains.Annotations.InstantHandleAttribute.#ctor" />     </parameter>   </member> </assembly>



Save the file, then reload the project - the warnings will now be gone!
Alternatively, you can place this file beside your Microsoft.Practices.TransientFaultHandling.Core.dll - named Microsoft.Practices.TransientFaultHandling.Core.ExternalAnnotations.xml.
This way you could distribute the annotations together with the file, in the source control perhaps, so that all team members will have the annotations as well!
0

Please sign in to leave a comment.