0

Here is pattern I often use to perform locks:

private IDisposable GetLock()
{
    _lock.Wait();
    return new Disposable(()=> _lock.Release());
}

The only problem I see in this code is Thread.Abort(), it can enter in between _lock.Wait() and return statement, leaving lock in incosistent state. I tried to avoid this by saving it into variable:

private IDisposable GetLock()
{
    var l = _lock;
    l.Wait();
    try
    {
       var result = new Disposable(()=> _lock.Release());
       l = null;       //<- I need to make this atomic, unabortable section
       return result;  //<- I need to make this atomic, unabortable section
    }
    finally
    {
       l?.Release();
    }
}

But abortation will still deadlock this section sometime. How one could accomplish protection of this section against Thread.Abort()?

eocron
  • 6,885
  • 1
  • 21
  • 50
  • You just reinvented/coded out the lock block. – Christopher Nov 09 '19 at 10:12
  • File classes implement IDisposeable. You release it by calling dispose. Same reinvention, just using instead of lock. – Christopher Nov 09 '19 at 10:14
  • The Database is responsible for managing any connections it opens. Latest on timeout, it will do that. You requirements are not clearly communicated in the least. – Christopher Nov 09 '19 at 10:24
  • Just so I am sure I get you right. Your asume that somebody - for no sensible reason - would write code for the sole purpose of sabotaging your code. And it would do that sabotage by inserting a Exception between the Null Check and dispose call? And all that would happen during release, for no discernible reason? | Because that level of sabotage, is not in your hand. You might as well try to protect your code from power outages or faulty RAM. – Christopher Nov 09 '19 at 10:52
  • So your idea is that ThreadAbort could be raised during the finally block and that there is no protection against this wierd scenario? – Christopher Nov 09 '19 at 10:59
  • If the exception happens in the try block, then the finally is called and there is no problem. Try has two simple jobs: a) run catch blocks on exceptions b) run finally in any case. That is **why** we put cleanup code into finally. Because. It. Runs.. Finally has that. one. job. – Christopher Nov 09 '19 at 11:07
  • I asked this question. Let us see what the people in the proper groups for such a question have to say on the mater. https://stackoverflow.com/questions/58778645/can-threadabort-be-raised-in-the-middle-of-a-finally-block – Christopher Nov 09 '19 at 11:10
  • I trusts answers on this thematic wenn someone like Erich Lippert has answered. The whole excption handling process is as cleanly regulated as the GC. Considering teh ThreadAbortException is odd in that it is re-raised after any try block (so it can not be swallowed), it would be *trivial* to have it queued for after the finally is done. Especialy since this is the kind of exception, that is most likely raised via a polling approach. – Christopher Nov 09 '19 at 11:20
  • If you write a infinite loop in a finally block and it is not cut out by the JiT and similar optimisations, that block will never be left. It is one of those many ways in wich you can sabotage your own code. Not any different from a infitnie loop anywhere else. Unless the OS or power outage kils the whole process - all threasd included - of coruse but again, then it is out of your hand. Thread.Abort follows the nice, *ordered* way of Exception handling laid out by the try block. There is decades to centuries (decades over a lot of people) of programming experience in the .NET Framework. – Christopher Nov 09 '19 at 13:16
  • Thread.Abort() is exceptionally nice for an Exception, even. In that the aborting the abort is entirely intended, as much as any other Multtiasking cancelation. Or even as much as Form.Closing() can cancel closing. There are definetily ways to kill the thread, wich would skip the finally. But we are not talking about such savage measures. – Christopher Nov 09 '19 at 13:18
  • finally will only protect you from the ThreadAbortedException. And only because it is not thrown/caught in the normal ways. Other Exceptions - especially the NullReference ones - can still break you out of the finally. But if you got that kind of exception, it breaking the finally is the least of your worry: You got a bug in your Exception Handling. That is the worst kind of bug. – Christopher Nov 09 '19 at 15:11
  • Your understanding of Exception might be flawed on a basic Level. There are two articles I consider basically "Must reads" for Exception handling. Usually I link them earlier, but here goes: https://blogs.msdn.microsoft.com/ericlippert/2008/09/10/vexing-exceptions/ | https://www.codeproject.com/Articles/9538/Exception-Handling-Best-Practices-in-NET | Especially the 1st where you classify wich exception to even catch, is very important. – Christopher Nov 09 '19 at 15:12
  • You have still not given a specific problem description past the "ThreadAbortException can cancel Finalizer". Wich turned out to be wrong. – Christopher Nov 09 '19 at 16:19
  • You have however talked about: Locks. File Handles. Databases. Requesting Microsoft to delete something from teh Framework/Language (of wich I still do not know what it is). Then teh whole "ThreadAbortException can cancel Finalizer" dragged itself over 2 threads and it turned out to be a wrong asumption on your end. – Christopher Nov 09 '19 at 16:27
  • I got the idea from this post: "No. Thread.Abort can inject into finally block too and you will lose your SQL connections, File handles and etc. There is no protection against Thread.Abort at all." If that is wrong what *IS* your problem actually? – Christopher Nov 09 '19 at 16:40
  • Now you are talking about managed and unmanaged resources all of a sudden. If you keep changing your problem, there is no way to help you. – Christopher Nov 09 '19 at 16:47

1 Answers1

1

You should never use Thread.Abort(). It mostly implies bad code structure. The only exception is that if you MUST use a buggy third-party library (which code you cannot change), you could terminate hanging code.

Let go the Thread.Abort() and your life will be much easier.

Just use the lock statement, which is fine.

It's a common rule that aborting a thread causes arbitrary behaviour and should be avoided at all times.

Jeroen van Langen
  • 21,446
  • 3
  • 42
  • 57
  • 1
    I know, it is sad, but if you write API, no one protecting you from this. Someone using your code, and you just want to write lock primitve. If it is bad primitve which deadlocks because of how system developed - no one will use it. – eocron Nov 09 '19 at 10:30
  • Sure, but you shoudn't fix problems that don't exists. You are not responsible for the ignorance or inexperienced of the developer that uses your API. It's a common rule that aborting a thread causes arbitrary behaviour and should be avoided at all* times. – Jeroen van Langen Nov 09 '19 at 10:35
  • If the need of aborting an action is desirable, you should build one in. Like setting a [`WaitHandle`](https://learn.microsoft.com/en-us/dotnet/api/system.threading.waithandle?view=netframework-4.8) which you will check or [`CancellationToken`](https://learn.microsoft.com/en-us/dotnet/api/system.threading.cancellationtoken?view=netframework-4.8) – Jeroen van Langen Nov 09 '19 at 10:36
  • Yeah, but there is question then, why deadlock doesn't happen in Mutex.Enter() which is ofcourse primitive written some way to avoid this on abort. In regard to possible Thread.Abort(). – eocron Nov 09 '19 at 10:38
  • Just a google: http://www.interact-sw.co.uk/iangblog/2004/11/12/cancellation – Jeroen van Langen Nov 09 '19 at 10:42
  • Actually it is possible to protect from Thread.Abort() -> try{}finally{/*protected section*/}. This section happens to not be interrupted in resent versions of .NET. – eocron Nov 09 '19 at 16:50