Using statement vs. thread abort not calling Dispose

   —   
I have come across solving a problem around using statement and locks that I believe is worth sharing with everyone. Short version: Be REALLY careful when using the using statement ... Long version: See the article ...
Hi there,

Read carefully, there is a behavior of C# using statement that you may not be aware of, and may cause you trouble even when debugging.

It all started when my coleague started to experience strange behavior of our asynchronous log control that he used for improved import of Newsletter subscribers in our online marketing solution in Kentico CMS v8. But that is another story, keep track on what is going on in v8 on our Twitter accounts if you are interested ...

The thing was, that when he cancelled the bulk delete thread, and re-run it again, the next thread got stuck for some unknown reason.

Stuck.png

After some debugging, we figured out that it most likely has something to do with new handlers implementation. A great thing about handlers in Kentico CMS v8 is that they:
  • Natively support transactions which starts in Before, and ends in After, there is no more need to do that with extra "hacks".
  • Natively support locks, so you can easily avoid multiple executions of the same handler if the process provided by this handler is considered critical section.
  • Natively support After callbacks registered in Before, which means that you can do some actions in the Before event, and then finish it in the After event, and pass the parameters across, no matter what happens in between those.
Let me just give you a small teaser here so you can see how neat it is:

public static void Init() { ObjectEvents.Delete.Before += Delete_Before; } private static void Delete_Before(object sender, ObjectEventArgs e) { var obj = e.Object; var genObj = obj.Generalized; // Use lock e.Lock(genObj.GetLockObject()); // Use transaction e.UseTransaction(); // Allow caching of the parent data, restore on dispose var originalCache = genObj.CacheParentData; e.CallOnDispose(() => genObj.CacheParentData = originalCache); genObj.CacheParentData = true; return obj; }


The original idea was that it has something to do with the related DB transaction. BTW, did you know that thread abort on open SQL transaction forcibly closes the open connection, letting you empty-handed with your rollback code? After some investigation, we figured out that we already handled that in previous versions, so the problem must be somewhere else.

Then we spent another hour by debugging and stepping through the code one line after another, until we figured out that the thread got stuck on unlocked lock! That is where it became really strange because all unit tests on this code passed when it was implemented, and the locks were always unlocked, because this handler was disposed within the using statement, so even if inner code raised exception, it should have been handled, right?

The worst thing that may ever happen to you that needs to be fixed in the C# code is a multithreading issue. It always is hard to reproduce, and also hard to find out the cause.

There was a Microsoft project called CHESS presented on PDC 2008, which was able to "unit test" multithreading issues, and seemed very promising, but unfortunately this project ended before it made it to public, and the original version of it's test adapter is not compatible with VS 2012. If you know about any alternative or in general what are the best practices to unit test multithreaded operations, please let me know ...

Anyway, after some Googling, I have luckily found this article which helped us a lot:

http://www.bluebytesoftware.com/blog/PermaLink.aspx?guid=c1898a31-a0aa-40af-871c-7847d98f1641

The problem is, that the using statement on its own is an atomic operation, BUT the code inside it isn't, and you may get in trouble if you are not really careful with it.

The problem is that if the initialization code inside using allocates some resources, or enters some locks, but the using statement for whatever reason, such as thread abort (in our case) or perhaps even regular exception, doesn't get the result, it doesn't force calling Dispose while the exception is bubbling up.

It is very hard to realize something like this may happen. Because at the first sight, the using statement seems OK. This is how we typically initiate event handlers:

// Handle the event using (var h = TypeInfo.Events.Delete.StartEvent(this)) { ... h.FinishEvent(); }


In our case, the handler within it's initialization executes the Before event, which is:
  • Opening transaction
  • Acquiring a lock on the given object sync key to prevent multiple executions of the same process (in our case, logging the staging task and some other synchronization work)
In case the handler properly finishes, it:
  • Commits transaction
  • Releases the lock
And in case of error in any part of the code (basically on Dispose if FinishEvent was not called):
  • Rollbacks transaction
  • Releases the lock
And this is where the stuck thread came from. The using statement didn't pick up the object, therefore it didn't call Dispose, therefore it didn't release the lock ... end of story.

How to solve such problem? Just follow one simple rule:

Never ever trust the using statement in releasing resources for you, and no matter how reliable the allocation of resources or shared locks is, always implement error handling around it that releases the resources in case of error, because thread abort exception is an asynchronous exception which can occur anytime whether you want it or not, and makes your seemingly reliable code unreliable for that instant moment.

I wrote this to share the knowledge, I believe that most of the developers are not aware of this potential problem as wasn't I before we faced it.

Feel free to share it to your friends to save everyone's time.

P.S.: I intentionally focused on covering most of the keywords and phrases related to this problem (mostly the ones I searched before I was able to find the solution), if you feel I missed some, go ahead and add them via comments so it is easier for others to find the solution if they face the same problem. Thanks!

Share this article on   LinkedIn

Martin Hejtmanek

Hi, I am the CTO of Kentico and I will be constantly providing you the information about current development process and other interesting technical things you might want to know about Kentico.

Comments

Nicklas Damgren commented on

I was thinking about creating a lock utilizing the using-statement, and while googling I've found tons of "recommended" and "smart" locks using the using-statement. I'm really glad I found this blog post too!

So I guess I will make it like:
using (var l = new UsingLock())
{
. . . l.Lock();
. . . // critical code goes here...
}

it's not perfect, it depends on how secure your code needs to be (mine isn't a service or server software)
...but for a simple user application I think this will do.
(If you forget to lock inside the using you will get an exception trying to unlock it, but invalid state has probably already happened.) (Assuming non-recursive locking.)

A good thing though is that this can easily be made (quite) secure since the Lock()-method should always be called on the first line inside the using-statement - this means that it's easy to create a test that finds all using-statements containing a "UsingLock" and that the next line locks it.

(So good testability and a readability improvement since all locking code is on two adjacent lines.)

This is off the top of my head though... I haven't yet decided if I really like this approach.