race conditions caused by Thread.Abort

  • Thread starter Thread starter Bric
  • Start date Start date
B

Bric

I have been looking for ways to prevent asynchronous
Thread.Abort calls from wreaking havoc in my code. I'm
working on an assembly, implemented in Managed Extensions
for C++, that makes unmanaged calls and often needs to
free resources associated with those calls:

bool allocated = false;
try
{
...
// Allocate some resources in unmanaged code
allocated = AllocateUnmanagedResources();
...
}
__finally
{
// Only free resources if allocation was successful
if (allocated)
FreeUnmanagedResources();
}

This code should work fine if the thread running it is not
aborted. However, asynchronously aborting the thread can
lead to two problems:
- The ThreadAbortException can be thrown from inside the
__finally block, causing the resources to be leaked. This
possibility is noted in the Thread.Abort documentation.
- The ThreadAbortException can be thrown after a
successful call to AllocateUnmanagedResources but before
the assignment to the boolean variable. In that case, the
code in the __finally block isn't aware of the current
state, so the resources are leaked.

Both of these scenarios seem to be the result of
deficiencies in the .NET Framework and/or CLR:
1) __finally blocks aren't guaranteed to run to completion
2) There doesn't seem to be a way to create
an "unabortable" block of code. Obviously, this kind of
feature could be abused (and would preclude the need for
(1) above), but it seems necessary in order to allow clean-
up code to know the state of the application.

Anyway, I'm looking for ways to eliminate the race
conditions described above. So far, the only solution
I've found is to surround the whole block with a
try/catch, and push the state variable assignment into
unmanaged code (which can't be aborted):

#pragma unmanaged
// Error-checking ignored for clarity
void AllocationWrapper(bool* pAllocated)
{
*pAllocated = AllocateUnmanagedResources();
}
void DeallocationWrapper(bool* pAllocated)
{
if (*pAllocated)
{
FreeUnmanagedResources();
*pAllocated = false;
}
}
#pragma managed

bool allocated = false;
try
{
try
{
...
AllocationWrapper(&allocated);
...
}
__finally
{
DeallocationWrapper(&allocated);
}
}
catch (ThreadAbortException*)
{
DeallocationWrapper(&allocated);
}

Can anyone think of a cleaner way to remove the race
conditions? Something in the language or framework I've
missed? A configuration setting that magically disables
Thread.Abort?

Thanks...
 
I agree that this is a nasty little surprise.

The simplest means of avoiding the problem is to not abort another thread.
I know that sounds silly but it may be a mechanism you can make use of. You
could write a library class that wrapped all allocations within a helper
method. In that method spawn another thread (not exposed to outside callers)
and do the allocation and assignment within that worker thread. Even if the
original thread the method was invoked on was aborted the worker thread
should still run to completion. You would need to synchronize the invoking
thread with the worker thread so that it blocked until all the allocations
were complete, but that is easy to do.

This still wont protect you from the situation where one piece of code is
invoking the allocation routine concurrently with another code section
causing the appdomain to get unloaded. Unloading an appdomain causes an
AbortException to be injected into all the threads within that appdomain.
But do you really need to protect yourself against schizophrenic application
code? And if the application is being unloaded would that eliminate the need
to release those resources anyway?

Dave
 
Thanks for the suggestions. I should mention that the
main reason this is a problem is the fact that ASP.NET
uses Thread.Abort to implement script timeouts (the
executionTimeout setting under the httpRuntime node). My
library can be used in other environments, but it's
primarily called from ASP.NET applications.

Obviously, an easy way to work around this is to make sure
that the executionTimeout setting is set high enough that
requests aren't timing out. However, I was also hoping to
find a way to avoid these problems through code, since
administrators are free to change the timeout setting.

Your suggestion of doing the unmanaged operations on a
separate thread makes sense. Because of the way our code
is structured (as a class library with these unmanaged
calls scattered throughout the implementation), that
solution would require some fairly radical changes, so I'm
not sure whether we'll attempt to resolve it that way.
It's worth considering, though. In the end, we may adopt
some sort of a compromise fix that avoids serious
instability but cannot prevent resource leaks. In the
meantime, I may try to open a case with Microsoft to see
what they're doing from their end...
 
Back
Top