FMultiReaderSingleWriterGT isn't thread safe

Hi guys,

I’ve found that FMultiReaderSingleWriterGT has at least 2 bugs:

The one that caused a deadlock in our game and caused me to investigate was this:

  1. Worker Thread Acquires Read lock
  2. Game Thread Acquires Read lock (Steps 1 and 2 can happen in either order)
  3. Worker Thread Releases Read lock
  4. Game Thread Releases Read lock
  5. Game Thread attempts to acquire Write lock (Deadlocks here)

This is deadlocked because ReadCounter is 0, but Action is left set as ReadingAction when the game thread releases the lock. Removing the IsInGameThread() guards in LockRead() and UnlockRead() would fix this.

I also noticed there is also a more standard problem that occurs with these kinds of lock, that the control data isn’t guarded, so you can get two threads modifying the lock in this pattern:

  1. Thread 1 - In LockRead Sets action to ReadingAction and Increments ReadCounter to 1
  2. Thread 2 - In Lock Read Sets ReadingAction,
  3. Thread 1 - In UnlockRead Decrements ReadCounter to 0 and sets action to NoAction
  4. Thread 2 - Increments ReadCounter to 1

Result: Thread 2 is reading, but lock is set to NoAction.

We’re on UE4.10, but a quick look seems to indicate neither of these have been fixed in the latest version of the engine…

As this is only used in ModuleManager, we’re opting to make ModuleManager use a FCriticalSection for now instead, but having a Multi-Reader, Single Writer lock would be useful in other scenarios.

Thanks.

Hi David,

Thanks for reporting this! I’ll make sure it gets fixed soon!

Looks like I’m having the issue here with wwise module and async loading.

Is the critical section the good way to go? It seems to indeed fix my issue:

What I’m seeing is that the module itself is destriyed because bad refcounter (I guess I’m in the second case David describes as I can see 2 concurrent reads).

By the way I’m on 4.12.5

For reference, I have entered this as UE-30929 in JIRA

Olivier - I replaced it with a critical section on our side as well, CL #3214800.

Thx for the reply!

I also needed cl3152273