Recursive use of non-recursive std::mtuex in Scheduler::ProcessTaskScheduling?

[Jan-Marek Glogowski]

Am 09.02.21 um 15:56 schrieb Stephan Bergmann:
> I occasionally see soffice.bin crash during `make check` tests on 
> Windows, and now building with --enable-breakpad, 
> JunitTest_forms_unoapi_1 failed with
> 

...

>> vclplug_winlo.dll!WinSalTimer::ImplHandleElapsedTimer() Line 166
>>     at C:\lo-clang\core\vcl\win\app\saltimer.cxx(166)
>> vclplug_winlo.dll!ImplSalYield(const bool bWait, const bool 
>> bHandleAllCurrentEvents) Line 456
>>     at C:\lo-clang\core\vcl\win\app\salinst.cxx(456)
>> vclplug_winlo.dll!SalComWndProc(HWND__ *, unsigned int nMsg, unsigned 
>> __int64 wParam, __int64 lParam, bool & rDef) Line 570
>>     at C:\lo-clang\core\vcl\win\app\salinst.cxx(570)
>> vclplug_winlo.dll!SalComWndProcW(HWND__ * hWnd, unsigned int nMsg, 
>> unsigned __int64 wParam, __int64 lParam) Line 655
>>     at C:\lo-clang\core\vcl\win\app\salinst.cxx(655)
>> user32.dll!UserCallWinProcCheckWow()
>> user32.dll!DispatchClientMessage
()
>> user32.dll!__fnDWORD
()
>> ntdll.dll!KiUserCallbackDispatcherContinue
()
>> user32.dll!NtUserPeekMessage
()
>> user32.dll!PeekMessageW
()
>> vclplug_winlo.dll!WinSalInstance::AnyInput(VclInputFlags nType) Line 801
>>     at C:\lo-clang\core\vcl\win\app\salinst.cxx(801)
>> vcllo.dll!Application::AnyInput(VclInputFlags nType) Line 590
>>     at C:\lo-clang\core\vcl\source\app\svapp.cxx(590)
>> vcllo.dll!Scheduler::ProcessTaskScheduling() Line 436
>>     at C:\lo-clang\core\vcl\source\app\scheduler.cxx(436)
> 
> ^ that frame has the same SchedulerGuard locked that the other 
> Scheduler::ProcessTaskScheduling frame further up wants to lock.

That is the new code from https://gerrit.libreoffice.org/c/core/+/110538

I think we have to revert that. I don't see a way to prevent AnyInput / 
PeekMessage to process messages. There is GetQueueStatus, but I guess 
that's not possible to be used correctly with our complicated user 
WM_USER based setup.

I wasn't anymore aware, that PeekMessage, called from AnyInput, will 
actually dispatch messages (well - the API docs start with "Dispatches 
incoming sent messages"). This backtrace just dispatches 
SAL_MSG_THREADYIELD in a nested scheduler loop.

Even with a recursive lock, this breaks the Scheduler event sequence, as 
it would now probably process an event before the current event.

IMHO the whole SAL_MSG_THREADYIELD should not exist, and we should have 
per-thread event loops and these wouldn't idle on the main thread at 
all, but that is probably a larger change, requiring a lot of new code 
to sync events with the main thread.