[Bug 54346] New: (Multithreaded) Applications sometimes get heap corruption on exit due to ignoring critical sections in Wine

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=54346

Bug ID: 54346
           Summary: (Multithreaded) Applications sometimes get heap
                    corruption on exit due to ignoring critical sections
                    in Wine
           Product: Wine
           Version: unspecified
          Hardware: x86-64
                OS: Linux
            Status: NEW
          Severity: normal
          Priority: P2
         Component: dmusic
          Assignee: [e-mail]
          Reporter: [e-mail]
      Distribution: ---

(Multithreaded) Applications sometimes get heap corruption on exit due to
ignoring critical sections in Wine. For instance dmloader:loader:

0118:warn:sync:RtlpWaitForCriticalSection process
L"Z:\home\fgouget\wine\wine-gitlab\dlls\dmloader\tests\i386-windows\dmloader_test.exe"
is shutting down, returning STATUS_SUCCESS
...
0118:warn:sync:RtlpWaitForCriticalSection process
L"Z:\home\fgouget\wine\wine-gitlab\dlls\dmloader\tests\i386-windows\dmloader_test.exe"
is shutting down, returning STATUS_SUCCESS
0118:err:sync:RtlLeaveCriticalSection section 00140074 "dlls/ntdll/heap.c: main
process heap section" is not acquired

Normally this does not cause the test to fail. But when running with
WINEDEBUG=heap this frequently leads to heap corruption because:
* Each call to the heap API triggers a heap validation.
* Each heap_validate() call is requires taking the main process heap lock.
* More contention makes the "not acquired" events more likely.
* Leading to multiple threads manipulating the heap at the same time, thus
causing corruption.

Also, be aware that when heap corruption is detected due to WINEDEBUG=heap,
Wine raises an exception which kills the process.

Also note that although dmloader:loader looks like it is not multithreaded (no
CreateThread() call), it loads dlls that create their own thread:
* winealsa.drv/midi.c -> notify_thread
* winealsa.drv/mmdevdrv.c -> alsa_timer_thread
* winepulse.drv/mmdevdrv.c -> pulse_mainloop_thread, pulse_timer_cb
...etc.

So this issue probably impacts most audio / multimedia applications and Wine
tests.

The "main process heap section is not acquired" errors most often look like
they are caused by the following functions which are all called on
DLL_PROCESS_DETACH:
* msacm32.MSACM_WriteCache() calling RegSetValueExA()
* ucrtbase.msvcrt_free_io() calling DeleteCriticalSection() when WINEDEBUG=heap

Ignoring the critical sections during shutdown is intentional and was
introduced by the 7def0f200f11 commit to fix bug 42470. See
RtlpWaitForCriticalSection():

/* Don't allow blocking on a critical section during process termination */
    if (RtlDllShutdownInProgress())
    {
        WARN( "process %s is shutting down, returning STATUS_SUCCESS\n",

debugstr_w(NtCurrentTeb()->Peb->ProcessParameters->ImagePathName.Buffer) );
        return STATUS_SUCCESS;
    }

However that commit also had to add todo_wine statements to kernel32:loader so
it's not clear that is is correct.