[Libreoffice-commits] core.git: vcl/inc vcl/osx vcl/README.scheduler vcl/win

[Jan-Marek Glogowski]

 vcl/README.scheduler |   51 +--
 vcl/inc/osx/saltimer.h   |3 --
 vcl/inc/saltimer.hxx |   44 
 vcl/inc/win/saltimer.h   |7 +++---
 vcl/osx/saltimer.cxx |   22 +---
 vcl/win/app/saltimer.cxx |   18 ++--
 6 files changed, 98 insertions(+), 47 deletions(-)

New commits:
commit da5cdcdeddf7bc21606b4cb64d8b1fc412146935
Author: Jan-Marek Glogowski 
Date:   Fri Sep 29 21:02:17 2017 +0200

Convert tick-based timer events to versioned ones

Instead of storing the system ticks in the timer event message
simply store a version.

Moves the version handling code into a VersionedEvent class,
inherited by WinSalTimer and AquaSalTimer.

Change-Id: I5add85031d36b3424a26a9ef798294cbfb00b2e4
Reviewed-on: https://gerrit.libreoffice.org/42959
Tested-by: Jenkins 
Reviewed-by: Jan-Marek Glogowski 

diff --git a/vcl/README.scheduler b/vcl/README.scheduler
index ac4a0dd698d4..8c5e64ba74c5 100644
--- a/vcl/README.scheduler
+++ b/vcl/README.scheduler
@@ -107,7 +107,7 @@ thread redirects using Qt::BlockingQueuedConnection.
 == General: processing all current events for DoYield ==
 
 This is easily implemented for all non-priority queue based implementations.
-Windows and MacOS both have a timestamp attached to their events / messages,
+Windows and macOS both have a timestamp attached to their events / messages,
 so simply get the current time and just process anything < timestamp.
 For the KDE backend this is already the default behaviour - single event
 processing isn't even supported. The headless backend accomplishes this by
@@ -130,10 +130,27 @@ may block the main thread until some events happen.
 Currently we wait on an extra conditional, which is cleared by the main event
 loop.
 
-== MacOS implementation details ==
+== General: invalidation of elapsed timer event messages ==
+
+Since the system timer to run the scheduler is single-shot, there should never
+be more then one elapsed timer event in system event queue. When stopping or
+restarting the timer, we eventually have to remove the now invalid event from
+the queue.
+
+But for the Windows and macOS backends this may fail as they have delayed
+posting of events, so a consecutive remove after a post will actually yield no
+remove. On Windows we even get unwanted processing of events outside of the
+main event loop, which may call the Scheduler, as timer management is handled
+in critical scheduler code.
+
+To prevent these problems, we don't even try to remove these events, but
+invalidate them by versioning the timer events. Timer events with invalid
+versions are processed but simply don't run the scheduler.
+
+== macOS implementation details ==
 
 Generally the Scheduler is handled as expected, except on resize, which is
-handled with different runloop-modes in MacOS. In case of a resize, the normal
+handled with different runloop-modes in macOS. In case of a resize, the normal
 runloop is suspended in sendEvent, so we can't call the scheduler via posted
 main loop-events. Instead the scheduler uses the timer again.
 
@@ -145,7 +162,7 @@ but we can prevent running any other SolarMutex based code. 
Those wakeup
 events must be ignored to prevent busy-locks. For more info read the "General:
 main thread deferral" section.
 
-We can neither rely on MacOS dispatch_sync code block execution nor the
+We can neither rely on macOS dispatch_sync code block execution nor the
 message handling, as both can't be prioritized or filtered and the first
 does also not allow nested execution and is just processed in sequence.
 
@@ -153,29 +170,25 @@ There is also a workaround for a problem for pushing 
tasks to an empty queue,
 as [NSApp postEvent: ... atStart: NO] doesn't append the event, if the
 message queue is empty.
 
-Probably that's the reason, why some code comments spoke of lost events and
-there was some distinct additional event processing implemented.
-
 == Windows implementation details ==
 
 Posted or sent event messages often trigger processing of WndProc in
 PeekMessage, GetMessage or DispatchMessage, independently from the message to
 fetch, remove or dispatch ("During this call, the system delivers pending,
 nonqueued messages..."). Additionally messages have an inherited priority
-based on the function used to generate them. Even if WM_TIMER should have been
-the lowest prio, a posted WM_TIMER is processed with the prio of a posted
-message.
+based on the function used to generate them. Even if WM_TIMER messages should
+have the lowest priority, a manually posted WM_TIMER is processed with the
+priority of a PostMessage message.
 
-Therefore the current solution always starts a (threaded) timer even for the
-instant Idles and syncs to this timer message in the main dispatch loop.
-Using SwitchToThread(), this seem to work 

[Libreoffice-commits] core.git: vcl/inc vcl/osx vcl/README.scheduler vcl/win

[Jan-Marek Glogowski]

 vcl/README.scheduler|8 
 vcl/inc/osx/salinst.h   |2 ++
 vcl/osx/salinst.cxx |   23 +--
 vcl/win/app/salinst.cxx |   28 +++-
 4 files changed, 34 insertions(+), 27 deletions(-)

New commits:
commit ce8bbb782b806e429ffb44226162967bed244d94
Author: Jan-Marek Glogowski 
Date:   Tue Aug 29 09:40:01 2017 +0200

Don't wait-yield non-main threads in the main thread

This prevents blocking the main thread by a yielding non-main thread.
The current solution is to wait on a condition, which is set by the
main thread on wakeup.

Change-Id: I8d680bb51a36ce1e0d3d4713d47d8e2ef93d7297
Reviewed-on: https://gerrit.libreoffice.org/42808
Tested-by: Jenkins 
Reviewed-by: Jan-Marek Glogowski 

diff --git a/vcl/README.scheduler b/vcl/README.scheduler
index 7e0d6ca8b467..ac4a0dd698d4 100644
--- a/vcl/README.scheduler
+++ b/vcl/README.scheduler
@@ -122,6 +122,14 @@ basically the same we're doing with the LO scheduler as a 
system event.
 The gen X11 backend has some levels of redirection, but needs quite some work
 to get this fixed.
 
+== General: non-main thread yield ==
+
+Yielding from a non-main thread must not wait in the main thread, as this
+may block the main thread until some events happen.
+
+Currently we wait on an extra conditional, which is cleared by the main event
+loop.
+
 == MacOS implementation details ==
 
 Generally the Scheduler is handled as expected, except on resize, which is
diff --git a/vcl/inc/osx/salinst.h b/vcl/inc/osx/salinst.h
index 65db0d0b0f2a..0e30dfafb693 100644
--- a/vcl/inc/osx/salinst.h
+++ b/vcl/inc/osx/salinst.h
@@ -72,6 +72,8 @@ class AquaSalInstance : public SalInstance
 {}
 };
 
+bool RunInMainYield( bool bHandleAllCurrentEvents );
+
 public:
 SalYieldMutex*  mpSalYieldMutex;// 
Sal-Yield-Mutex
 OUStringmaDefaultPrinter;
diff --git a/vcl/osx/salinst.cxx b/vcl/osx/salinst.cxx
index e3e101cbc13e..f55e9b4aa297 100644
--- a/vcl/osx/salinst.cxx
+++ b/vcl/osx/salinst.cxx
@@ -525,6 +525,13 @@ void AquaSalInstance::handleAppDefinedEvent( NSEvent* 
pEvent )
 };
 }
 
+bool AquaSalInstance::RunInMainYield( bool bHandleAllCurrentEvents )
+{
+OSX_SALDATA_RUNINMAIN_UNION( DoYield( false, bHandleAllCurrentEvents), 
boolean )
+assert( false && "Don't call this from the main thread!" );
+return false;
+
+}
 static bool isWakeupEvent( NSEvent *pEvent )
 {
 SAL_WNODEPRECATED_DECLARATIONS_PUSH
@@ -645,13 +652,17 @@ SAL_WNODEPRECATED_DECLARATIONS_POP
 if ( bHadEvent )
 maWaitingYieldCond.set();
 }
-else if( bWait )
+else
 {
-// #i103162#
-// wait until the main thread has dispatched an event
-maWaitingYieldCond.reset();
-SolarMutexReleaser aReleaser;
-maWaitingYieldCond.wait();
+bHadEvent = RunInMainYield( bHandleAllCurrentEvents );
+if ( !bHadEvent && bWait )
+{
+// #i103162#
+// wait until the main thread has dispatched an event
+maWaitingYieldCond.reset();
+SolarMutexReleaser aReleaser;
+maWaitingYieldCond.wait();
+}
 }
 
 // we get some apple events way too early
diff --git a/vcl/win/app/salinst.cxx b/vcl/win/app/salinst.cxx
index 23e48532d3b9..b77e84eac739 100644
--- a/vcl/win/app/salinst.cxx
+++ b/vcl/win/app/salinst.cxx
@@ -572,31 +572,16 @@ bool WinSalInstance::DoYield(bool bWait, bool 
bHandleAllCurrentEvents)
 SolarMutexReleaser aReleaser;
 if ( !IsMainThread() )
 {
-if ( bWait )
+// If you change the SendMessageW function, you might need to update
+// the PeekMessage( ... PM_QS_POSTMESSAGE) calls!
+bDidWork = SendMessageW( mhComWnd, SAL_MSG_THREADYIELD,
+ (WPARAM) false, (LPARAM) 
bHandleAllCurrentEvents );
+if ( !bDidWork && bWait )
 {
 maWaitingYieldCond.reset();
 maWaitingYieldCond.wait();
 bDidWork = true;
 }
-else {
-// #97739# A SendMessage call blocks until the called thread 
(here: the main thread)
-// returns. During a yield however, messages are processed in the 
main thread that might
-// result in a new message loop due to opening a dialog. Thus, 
SendMessage would not
-// return which will block this thread!
-// Solution: just give up the time slice and hope that messages 
are processed
-// by the main thread anyway (where all windows are created)
-// If the mainthread is not currently handling messages, then our 
SendMessage would
-// also do nothing, so this seems to be reasonable.
-
-// #i18883# only sleep if potential deadlock scenario, ie, when a 
dialog is open
-if(