Hi,

I'm not sure what's going on here, looks like I'm having a deadlock for some reason, but I don't know why.

I have something like this:

// called inside an infinite loop in its own thread
void Render(){
  mutex.lock();
  // lots of stuff
  mutex.unlock();
}

// a Windows event like a button pressed, etc
void SomeEvent(){
  mutex.lock();
  // a few updates here and there
  mutex.unlock();
}

In some machines this runs without any problems, but on others when SomeEvent() gets called it gets stuck waiting for the lock, while Render() is still being called regularly inside its loop, locking() and unlocking() the mutex, as if there was no one else waiting for it.

This makes me wonder how the OS handles when someone is waiting for the mutex, is the waiting thread queued? or is it just polling the mutex every x time to see if it can lock the mutex until it succeeds? or what else is involved when a thread is waiting?

Any insight would be appreciated.

Thanks!

if u r talking about std::mutex, then try this:

• remove all your locks/unlocks and replace with:

 std::mutex your_mutex;
 
 void render()
 {
 	std::lock_guard<std::mutex> guard(your_mutex); 
 	... your render code ...
 }
 // do the same in the other function

have fun ?

@ddlox AFAIK that would give the same behaivor as locking/unlocking manually, since lock_guard would call unlock() on destruction; wouldn't it?.. not sure it would make any difference to fix the problem.

Besides, in my particular case I need to expose the lock/unlock to the UWP side of the application at least for the Event() functions.

*UPDATE*: well, I've just switched to lock_guard on the Render() function only and apparently its not getting into deadlock anymore… now I'm more curious of why could that be if the behavior should be the same as locking/unlocking manually.

jakovo said:
but on others … it gets stuck waiting

How do you test that? I came over some interesting cases where the OS behavior changes in between debug and release mode. It was possible to lock a mutex on the same thread in debug mode (to lock the thread out itself) but in release mode this passed through and the code didn't work as expected and testet in debug.

In general, the OS sets threads asleep when they run into an already locked mutex and assigns an event to them, which means that when the mutex gets unlocked, all events are fired and the threads awake trying to get the lock. However, the OS Scheduler prioritizes already running threads over those that were at sleep for a while. This ends into some milliseconds of delay from a lock released to a thread awaking and is totally unpredictable.

One scenario I can imagine in your case is that the OS is running the Render-Loop faster than any Event can get the lock and so it is missing that all the time. I don't know how you setup your Render-Loop and who is calling the Event function, maybe some code in Render causes Events to drop which will be a lockout. Or it is just the CRT which is somehow different or an Update that is installed or some Hardware-Tweaks. However, it gets really difficult to get into that without a good test-setup.

My suggestion, if you don't want to use the lock_guard, use the mutex and an additional signal which is set in any event call. So Render is doing it's thing and after you unlock everything, test the signal. If it is set, Render will automatically go to sleep for as long as Event is running

there's no definitive answer here, what u thought was a deadlock could have a been an Undefined Behaviour (UB) and we could talk about UBs for the rest of our lives;

it's not because the lock_guard workaround appears to work that u should think u have solved the issue;

the removal of lock/unlock calls and the intro of lock_guard might have shifted the real problem elsewhere;

having said all this, there are truths which are commonly known such as locking 2 or more mutexes in opposite orders (by multiple threads) causes a deadlock;

but there are many other realities which even the C++ committee define as Undefined Behaviours (UB), for example, when the same thread tries to lock the same mutex (that's also a UB and not a deadlock), and this could be what happened in your case or maybe not (here we go);

it gets even more interesting when exceptions are turned off;

the reason I suggested to use lock_guard is because even though u can call lock/unlock on a std::mutex it is not recommended by C++ code of practice;

std::mutex should not be used directly -as u did-, always try to use one of the mutex management offerings namely: lock_guard, shared_lock, scoped_lock, unique_lock… they are all different and have a specific meaning, but these are all designed to offer “better” mutex management ownership to the calling thread and exclusivity (i'm not 100% sure about all the mechanics underneath this but i have seen enough to believe that these RAII locks are better to use then the mutexes themselves);

so stick to these and things should be a bit better 4 u ?

that's it… all the best ?

How do you test that?

just setting a printf() before and after the lock. =)

Shaarigan said:
My suggestion, if you don't want to use the lock_guard, use the mutex and an additional signal which is set in any event call. So Render is doing it's thing and after you unlock everything, test the signal. If it is set, Render will automatically go to sleep for as long as Event is running

Nice!.. I definitely went with this approach, thanks!… now render() waits for any event waiting the mutex to be processed before starting again; there should only be events every now and then so that shouldn't affect performance.

ddlox said:
i'm not 100% sure about all the mechanics underneath this but i have seen enough to believe that these RAII locks are better to use then the mutexes themselves

I can see why using RAII is better than locking/unlocking manually, specially in the case of exceptions.. but (as far as documentation says) handling carefully should give the same behavior; but yeah.. probably need to get closer into the mechanincs underneath to better understand other, more subtle, ways the RAII locks could give further guarantees than doing it manually.

Thank you both for the info!

When exceptions are thrown, you should usually be warned by the CRT and the thread will break even if you turned them off so this should not be kind of an issue. I write C++ without any exceptions in general and always saw when something broke, regardless if it was debug or release code so trust the CRT!

Btw. are you sure you want to lock the entire Render loop? What are you doing that it isn't possible to fine grained lock specific critical resources you are using or could you even use atomic operations for some of them (like incrementing a variable, changing a pointer etc. )?

To be honest, I never ever needed a full lock in my game code so far when a simple spin locking was enougth or read/write locking in a lot of cases when resources where changed rarely but read often

template<typename T> force_inline void lock(volatile T* flag) 
{ 
    while (Interlocked::Exchange(flag, 1)) 
        while (*flag) 
        { }
}
template<typename T> force_inline void unlock(volatile T* flag)
{
    Interlocked::Exchange(flag, 0);
}

Shaarigan said:
When exceptions are thrown, you should usually be warned by the CRT and the thread will break even if you turned them off so this should not be kind of an issue.

What I meant is that if you manually lock the mutex, and then some exception is thrown, the thread will break and you won't be able to unlock the mutex, and so every other thread waiting for it will be stalled for ever. Using RAII the mutex will be unlocked automatically when the thread resources get released.

Shaarigan said:
are you sure you want to lock the entire Render loop? What are you doing that it isn't possible to fine grained lock specific critical resources you are using

Well, In the render loop I'm iterating over my renderables (in its own thread), and the xxEvent() functions might delete some (or all) of those renderables in another thread, so I need them to be deleted until the render loop has finished iterating over them; otherwise… well… you can imagine.

=)

jakovo said:
What I meant is that if you manually lock the mutex, and then some exception is thrown, the thread will break and you won't be able to unlock the mutex, and so every other thread waiting for it will be stalled for ever.

Sure but as I wrote above, the OS will prompt an exception dialog if you didn't ctahc anything and close the application (related to where the exception was thrown) so you won't anyway be able to further execute any code that may stall forever. Unless you built an exception handle into your code that will catch and raise those signals on a per-thread level. In this case you might be able to rescue your application but then yes, sure you will be stalling at some point.

jakovo said:
Well, In the render loop I'm iterating over my renderables

This sounds like the classic “an event handler is removing itself from the list of event handlers while they are processed” scenario. As it “may” but must not happen, go for the ReadWriteLock approach and you'll not just saving performance but also only work on those items if needed and not all the time and I guess it is ok for your events to spin-wait some time. I use those locks together with simple exclusive spin-locks all the time (battle.proof might be no the right workd but somehow) and they work well for me. Another approach I tried once I wrote my event handler to solve the issue above was to maintain two pointers to such a list at a time. One was used to add/remove handlers while the other was for processing. I simply copied one list into another when the processing was called again. I don't know if this is fast enougth for a render loop but it worked well in my event system