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

Jan-Marek Glogowski glogow at fbihome.de
Tue Feb 9 22:16:24 UTC 2021


Am 09.02.21 um 19:08 schrieb Noel Grandin:
> On Tue, 9 Feb 2021 at 18:13, Jan-Marek Glogowski <glogow at fbihome.de 
> <mailto:glogow at fbihome.de>> wrote: 
> 
> Looking at SAL_MSG_THREADYIELD, the primary use-case seems to be things 
> calling Application::Reschedule,
> which wants to wait until all the current events have been processed.

Would be nice, if that was true. This is platform specific code. For 
example gtk3 has "int nMaxEvents = bHandleAllCurrentEvents ? 100 : 1;".
Qt can't "practically" handle single events, so we call processEvents 
with QEventLoop::AllEvents, which "Processes pending events that match 
flags until there are no more events to process."

Over time I changed WIN to process system events from "true == 100" to 
"event timestamp < processing start time". And OSX from a 
"ProcessEventsToIdle" equivalent to do the same; for better or worse.

I would also like to drop the "bHandleAllCurrentEvents" parameter by 
defaulting to true. But that doesn't seem to be implementable by gtk 
without that "true == 100" hack, but maybe it's still better for 
consistency. OTOH Qt uses glib main loop internally and doesn't allow to 
restrict event processing in a sensible way, except timeout. You can't 
even check for pending events in Qt (deprecated). From all I see, gtk 
and Qt have conflicting requirements here.

> Surely that could be more easily implemented by logic like:
> 
>      if (we're on event thread)
>          handle_events()
>      else
>      {
>          SpecialWaitEvent aEvent;
>          Application::postUserEvent(&aEvent);
>          aEvent.waitUntilProcessedByEventThread();
>      }
> 
> which is largely the same logic, but without needing special handling 
> inside the scheduler.

So there is already Appplication::PostUserEvent + ImplSVEvent + 
SalUserEvent. But without anything like "waitUntilProcessedByEventThread".
That code depends on a backend specific SalFrame::PostEvent. AFAIK all 
backends except Windows use SalUserEventList to handle them. Windows 
uses PostMessageW(SAL_MSG_USEREVENT), which actually might be wrong.
SalUserEvents for SalUserEventList are processed in 
SalInstance::DoYield, even before any system events. But you must ignore 
your own event, otherwise you would just process yourself and report to 
have "done events" .

So you can't rely on any LO abstraction and would need to do something 
Windows specific. And there is the whole SalYieldMutex::doAcquire, 
MsgWaitForMultipleObjects, OSL condition, QS_SENDMESSAGE handling, for 
the m_nNoYieldLock "GUI processing redirect to main thread" SolarMutex 
workaround. So currently you must use SendMessage, otherwise a blocked 
main thread on acquiring the SolarMutex wouldn't process your message 
and you would unintentionally block, even if you don't want to wait (if 
commit ce8bbb782b806e429ffb44226162967bed244d94 is still needed).

Jan-Marek

P.S. this is all way more complex, then I like it. Feel free to give a 
variant of your idea a shot, but AFAIK your described approach wouldn't 
work. And the unit test coverage for this is mediocre, but hopefully 
make check would nevertheless fail on a breaking code variant.


More information about the LibreOffice mailing list