Wtf-ish multithreading?



  • Have a vb.net app that has around three or four event loops, kicked off by a timer, that could potentially be simultaneously running in their own threads. User might need to do something which requires any synchronized code be idle while in progress.

    What I want to do is acquire all the locks (if possible), perform the action, and then release them again so we haven't just permanently blocked one of the four loops from running again. I'm afraid that if, when trying to acquire any of the four locks, it will be unavailable and hence the rest of the locks should be released so as not to block the other threads just by virtue of trying to acquire their lock(s). This is the best I've come up with so far:

        Public Shared Function AcquireAllLocks() As Boolean
            If Not Monitor.TryEnter(Lock1) Then
                Return False
            ElseIf Not Monitor.TryEnter(Lock2) Then
                Monitor.Exit(Lock1)
                Return False
            ElseIf Not Monitor.TryEnter(Lock3) Then
                Monitor.Exit(Lock1)
                Monitor.Exit(Lock2)
                Return False
            ElseIf Not Monitor.TryEnter(Lock4) Then
                Monitor.Exit(Lock1)
                Monitor.Exit(Lock2)
                Monitor.Exit(Lock3)
                Return False
            Else
                Return True
            End If
        End Function
    

    I have a sneaking suspicion this is not the best way to do it. Advice?



  • The WTF here is that you need four locks to do something. You may want to rethink your architecture.

     

    Why do you need four locks? What are you doing to four things at once?



  • Yeah, inheriting a badly-conceived project is not my cup of tea, but I can see where the original author went all threadpool-happy and decided that would help performance.

    I'm trying to rejigger the thing to avoid deadlocks, spontaneous breakage, etc that users were getting before but I don't want to rip out the synchronized portions if I can avoid it.

    I can see a reason to do maybe 2 of the 4 things simultaneously that he's locking here. Those event loops need to process fairly large queues of data and fire off webservice calls, output file generation, etc where one event could take a fairly long time and ought not to block the other (e.g., one large queue running procedurally to the second queue would block the second from running for an unacceptably long time)...


     



  • @sootzoo said:

    Yeah, inheriting a badly-conceived project is not my cup of tea, but I can see where the original author went all threadpool-happy and decided that would help performance.

    I'm trying to rejigger the thing to avoid deadlocks, spontaneous breakage, etc that users were getting before but I don't want to rip out the synchronized portions if I can avoid it.

    I can see a reason to do maybe 2 of the 4 things simultaneously that he's locking here. Those event loops need to process fairly large queues of data and fire off webservice calls, output file generation, etc where one event could take a fairly long time and ought not to block the other (e.g., one large queue running procedurally to the second queue would block the second from running for an unacceptably long time)...

    I guess my question is "do you really need those locks?" Are they *really* dependant on each other?

    The reason I ask is that over the years I've noticed a tendency to *overthink* paralell processing. When you learn multithreading, the first thing you learn about, and the big thing that gets hammered at you is the need for a mutex or resource lock, but the thing that's rarely taught is when you *don't* need them. You shouldn't ever need a lock on *four* resources at the same time. You might occasionally need a resource lock on two resources, but that is the exception, not the rule. Four.... I can't see why. Which is why I asked why -- 'cause I'd really like to know of a *valid* situation where you need four locks.



  • If this is the only function that uses these four locks, then you obviously need only one lock.  This leads me to believe that each of the locks is used someplace else.  Maybe other places use two or more locks.

    I assume from what you said that each event loop, each in its own thread, uses one lock; i.e. "while true { wait for event, acquire lock, process event, release lock }".

    But if your code needs to acquire more than one lock, there's probably code somewhere else that needs to acquire more than one lock, so unless all these pieces of code acquire the locks in the correct order, you have a possibility for deadlock.

    You should have a clear idea of which lock controls which group of resources.  Can you break it down to non-exclusive parts so that you can acquire one lock, do something, release the lock, acquire the second lock, do something, release the lock, etc.?

    If this won't work because of an effect the second thing may have on the first thing, then you should consider looking at it as a "transaction" problem.  An analogous situation:  lock name table, update name, unlock name table, lock address table, try to update address, update fails on error, unlock address table, lock name table, undo name change, unlock name table, notify user of error.

     



  • Further thoughts:

    If there is absolutely no way you can clean up the design, then you should strongly consider writing the function AcquireAllLocks() to take a list of locks to be acquired, for instance only Lock2 and Lock4 if those are all that are needed.  Then find all code that acquires more than one lock and have it call your AcquireAllLocks(locklist), just so you can be sure the locks are aquired in the correct order.

    Even then, I don't like it much.  It does have the virtue of not blocking a thread while waiting for another thread's lock, but there's no guarantee it will ever acquire all locks.  (One of your Dining Philosophers may starve to death (if you don't know what "Dining Philosophers" are, look it up).)  What if Lock 1 is always held for the first 16 seconds of every minute, lock 2 for seconds 15 through 31 etc.?  The only possible way you can guarantee acquiring all locks is to wait for the first, wait for the second, etc.  This will hold up the thread that needs the first lock.

    Anyway, it's resources, not threads, that need locks.  Consider this:  each of your event loops processes an event relating to one resource.  Can you create an event and stick it into the event queue?  (In the Microsoft Windows API, this is like SendMessage(); in Java there is SwingUtilities.invokeAndWait().)

    So then can you break down your user request into a series of requests for the different event loops?  Like this:

       do something that requires no locks;

       DatabaseEventQueue.invokeAndWait(DatabaseCommand);

       do something else that requires no locks;

       SomeOtherEventQueue.invokeAndWait(SomeOtherCommand);

    Then the user request is guaranteed to get done, one step at a time, all in due course of running the event queues.

    I don't know VB.NET or the CLR, but the event queues must have the equivalent of SendMessage() or invokeAndWait() for putting an event in the queue and waiting until the queue has processed it.  If not, I can tell you how you could implement it.

     



  • @newfweiler said:

    Can you break it down to non-exclusive parts so that you can acquire one lock, do something, release the lock, acquire the second lock, do something, release the lock, etc.?

    I think so. Thanks a bunch! 



  • @sootzoo said:

    @newfweiler said:
    Can you break it down to non-exclusive parts so that you can acquire one lock, do something, release the lock, acquire the second lock, do something, release the lock, etc.?

    I think so. Thanks a bunch! 

    Wait, isn't that what *I* said?

     

    Damn you newfweiler and your concisity!

     

    Is that a word?



  • @Whiskey Tango Foxtrot? Over. said:

    @sootzoo said:
    @newfweiler said:
    Can you break it down to non-exclusive parts so that you can acquire one lock, do something, release the lock, acquire the second lock, do something, release the lock, etc.?

    I think so. Thanks a bunch! 

    Wait, isn't that what *I* said?

     

    Damn you newfweiler and your concisity!

     

    Is that a word?

    You make it too easy for me!  All I have to do is read your reply and concisify it, and everyone things I'm really smart.

     



  • @newfweiler said:

    You make it too easy for me!  All I have to do is read your reply and concisify it, and everyone things I'm really smart.

     

    Then you misspel "think" and the illusion is shattered. :D



  • @Whiskey Tango Foxtrot? Over. said:

    @newfweiler said:

    You make it too easy for me!  All I have to do is read your reply and concisify it, and everyone things I'm really smart.

     

    Then you misspel "think" and the illusion is shattered. :D

    Yes, but I can blame it on the forum software.  I set the character set to "Canadian" and typed "think".  Yeah, that's it.

     



  • @newfweiler said:

    I don't know VB.NET or the CLR, but the event queues must have the equivalent of SendMessage() or invokeAndWait() for putting an event in the queue and waiting until the queue has processed it.  If not, I can tell you how you could implement it.

     

    Do tell :-) Or link!

    Prtyplz!



  • @GeneWitch said:

    @newfweiler said:

    I don't know VB.NET or the CLR, but the event queues must have the equivalent of SendMessage() or invokeAndWait() for putting an event in the queue and waiting until the queue has processed it.  If not, I can tell you how you could implement it.

     

    Do tell :-) Or link!

    Prtyplz!

    What you are going to do essentially is to have your main thread make some code be executed on an event queue thread.  Let's say for instance you have an event queue that gets data from a database.  So your main function is doing this:

    acquire database lock; // This makes the database event queue stop.
    x = database.get(); // This does the work.
    release database lock; // This lets the database queue go again.

    I assume the database thread works from a queue that contains commands, and you know how to put a command into the queue. So you know how to package up a command and put it into the queue to be eventually executed on the database thread. In other words, in the above code you can make "x = database.get()" execute on the database thread, but you need to somehow or other get x back.

    I don't know VB.NET, but I know it uses the CLR which has a Java-like look to it, so I'll assume that a command is a VB object that looks something like it would in Java. (I hope your application is already designed to do something like this; it will make it easier.) In Java you create a class that inherits from Runnable and has a method run(). So the code for the database thread should look something like this:

    Do While True
    If nothing in queue Then
    wait until something is in queue
    End If
    command = get head of queue
    command.Run() // includes code to acquire and release lock if necessary
    Loop

    So your main function creates an object that works like a command, puts it into the database queue, and the database thread executes it. Now, if this looks like what you already have, we can work on getting the result back.

    To do this, we need to use something called a "FutureResult" or "IOU". This is an object that has a value in it, along with some synchronizing code. One thread waits for the other thread to put the value into it. There is probably an object like this in the CLR that you can use. Here is what it looks like in pseudo-Java pseudo-code. I assume "Lock" is a very simple lock.

    class FutureResult {
    protected Object value = null; // Result goes here
    protected boolean ready = false;
    protected Lock lock1 = Lock(initially free); // Locks out other thread for short time.
    protected Lock resultLock = Lock(initially acquired); // Not released until ready.
    public boolean available() { // Return true if result ready
    acquire lock1;
    boolean result = ready;
    release lock1;
    return result;
    }
    public Object get() { // Wait for result and return it when ready
    while (!available()) {
    acquire resultLock; // Blocks here until ready
    }
    return value;
    }
    public void set(Object result) {
    acquire lock1; // Makes sure value and ready are set together, as seen by other thread
    value = result;
    ready = true;
    release resultLock;
    release lock1;
    }

    Your main function creates a command object that includes a FutureResult. It puts the object into the database queue and then calls "x = result.get()" on that result. (The resultLock has to be a non-recursive lock; i.e. it has to be a lock that blocks even though the same thread acquired the lock. The lock is set by one thread and released by the other thread.) Now this function is blocked until the database thread executes the command.

    The database thread gets the command, executes it, and then calls "result.set(x)". This releases resultLock only after setting the value. Then get() finishes running on the other thread so it returns the value.

     

    (Yeah, the formatting looks awful.  I still don't know how to use this editor properly.)

     


Log in to reply