vcl/headless/svpinst.cxx | 82 ++++++++-----------------------------------
vcl/inc/headless/svpinst.hxx | 9 +++-
2 files changed, 22 insertions(+), 69 deletions(-)
New commits:
commit 91526a2f53d5c7a703b9fc5fbc1728ee50854cc1
Author: Tor Lillqvist <t...@collabora.com>
AuthorDate: Thu Dec 29 12:16:25 2022 +0200
Commit: Tor Lillqvist <t...@collabora.com>
CommitDate: Mon Jan 2 17:57:16 2023 +0000
Use std synchronisation APIs instead of a pipe
The immediate reason for this is that pipes are broken in the
Emscripten runtime, see
https://github.com/emscripten-core/emscripten/issues/13214. But if we
can drop the use of a pipe for other platforms, too, why not.
Without this, when attemting to run Collabora Online as WASM, I get:
Aborted(Assertion failed: nRet == 1, at:
.../vcl/headless/svpinst.cxx,538,DoYield)
It is quite possible that the code could be simplified drastically. I
only replaced the use of a pipe with hopefully equivalent use of a
queue, a condition variable, and a mutex.
Change-Id: I9259ba36afeabce6474a1aec827d01bcbbd4412b
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/144944
Reviewed-by: Michael Meeks <michael.me...@collabora.com>
Tested-by: Jenkins
diff --git a/vcl/headless/svpinst.cxx b/vcl/headless/svpinst.cxx
index 5a86826423e3..19eef8997689 100644
--- a/vcl/headless/svpinst.cxx
+++ b/vcl/headless/svpinst.cxx
@@ -22,9 +22,6 @@
#include <mutex>
-#include <unistd.h>
-#include <errno.h>
-#include <fcntl.h>
#include <pthread.h>
#include <sys/time.h>
#include <poll.h>
@@ -76,14 +73,13 @@ do { \
#define DBG_TESTSVPYIELDMUTEX() ((void)0)
#endif
-#if !defined(ANDROID) && !defined(IOS)
+#if !defined(ANDROID) && !defined(IOS) && !defined(EMSCRIPTEN)
static void atfork_child()
{
if (SvpSalInstance::s_pDefaultInstance != nullptr)
{
- SvpSalInstance::s_pDefaultInstance->CloseWakeupPipe(false);
- SvpSalInstance::s_pDefaultInstance->CreateWakeupPipe(false);
+ SvpSalInstance::s_pDefaultInstance->CloseWakeupPipe();
}
}
@@ -97,10 +93,9 @@ SvpSalInstance::SvpSalInstance(
std::unique_ptr<SalYieldMutex> pMutex )
m_nTimeoutMS = 0;
m_MainThread = osl::Thread::getCurrentIdentifier();
- CreateWakeupPipe(true);
if( s_pDefaultInstance == nullptr )
s_pDefaultInstance = this;
-#if !defined(ANDROID) && !defined(IOS)
+#if !defined(ANDROID) && !defined(IOS) && !defined(EMSCRIPTEN)
pthread_atfork(nullptr, nullptr, atfork_child);
#endif
}
@@ -109,62 +104,16 @@ SvpSalInstance::~SvpSalInstance()
{
if( s_pDefaultInstance == this )
s_pDefaultInstance = nullptr;
- CloseWakeupPipe(true);
+ CloseWakeupPipe();
}
-void SvpSalInstance::CloseWakeupPipe(bool log)
+void SvpSalInstance::CloseWakeupPipe()
{
SvpSalYieldMutex *const
pMutex(dynamic_cast<SvpSalYieldMutex*>(GetYieldMutex()));
if (!pMutex)
return;
- if (pMutex->m_FeedbackFDs[0] != -1)
- {
- if (log)
- {
- SAL_INFO("vcl.headless", "CloseWakeupPipe: Closing inherited
feedback pipe: [" << pMutex->m_FeedbackFDs[0] << "," <<
pMutex->m_FeedbackFDs[1] << "]");
- }
- close (pMutex->m_FeedbackFDs[0]);
- close (pMutex->m_FeedbackFDs[1]);
- pMutex->m_FeedbackFDs[0] = pMutex->m_FeedbackFDs[1] = -1;
- }
-}
-
-void SvpSalInstance::CreateWakeupPipe(bool log)
-{
- SvpSalYieldMutex *const
pMutex(dynamic_cast<SvpSalYieldMutex*>(GetYieldMutex()));
- if (!pMutex)
- return;
- if (pipe (pMutex->m_FeedbackFDs) == -1)
- {
- if (log)
- {
- SAL_WARN("vcl.headless", "Could not create feedback pipe: " <<
strerror(errno));
- std::abort();
- }
- }
- else
- {
- if (log)
- {
- SAL_INFO("vcl.headless", "CreateWakeupPipe: Created feedback pipe:
[" << pMutex->m_FeedbackFDs[0] << "," << pMutex->m_FeedbackFDs[1] << "]");
- }
-
- int flags;
-
- // set close-on-exec descriptor flag.
- if ((flags = fcntl (pMutex->m_FeedbackFDs[0], F_GETFD)) != -1)
- {
- flags |= FD_CLOEXEC;
- (void) fcntl(pMutex->m_FeedbackFDs[0], F_SETFD, flags);
- }
- if ((flags = fcntl (pMutex->m_FeedbackFDs[1], F_GETFD)) != -1)
- {
- flags |= FD_CLOEXEC;
- (void) fcntl(pMutex->m_FeedbackFDs[1], F_SETFD, flags);
- }
-
- // retain the default blocking I/O for feedback pipe
- }
+ while (!pMutex->m_FeedbackPipe.empty())
+ pMutex->m_FeedbackPipe.pop();
}
void SvpSalInstance::TriggerUserEventProcessing()
@@ -328,9 +277,6 @@ void SvpSalInstance::ProcessEvent( SalUserEvent aEvent )
SvpSalYieldMutex::SvpSalYieldMutex()
{
-#ifndef IOS
- m_FeedbackFDs[0] = m_FeedbackFDs[1] = -1;
-#endif
}
SvpSalYieldMutex::~SvpSalYieldMutex()
@@ -367,11 +313,11 @@ void SvpSalYieldMutex::doAcquire(sal_uInt32 const
nLockCount)
m_bNoYieldLock = true;
bool const bEvents = pInst->DoYield(false, request ==
SvpRequest::MainThreadDispatchAllEvents);
m_bNoYieldLock = false;
- if (write(m_FeedbackFDs[1], &bEvents, sizeof(bool)) !=
sizeof(bool))
{
- SAL_WARN("vcl.headless", "Could not write: " <<
strerror(errno));
- std::abort();
+ std::lock_guard lock(m_FeedbackMutex);
+ m_FeedbackPipe.push(bEvents);
}
+ m_FeedbackCV.notify_all();
}
}
while (true);
@@ -534,8 +480,12 @@ bool SvpSalInstance::DoYield(bool bWait, bool
bHandleAllCurrentEvents)
: SvpRequest::MainThreadDispatchOneEvent);
// blocking read (for synchronisation)
- auto const nRet = read(pMutex->m_FeedbackFDs[0], &bWasEvent,
sizeof(bool));
- assert(nRet == 1); (void) nRet;
+ {
+ std::unique_lock lock(pMutex->m_FeedbackMutex);
+ pMutex->m_FeedbackCV.wait(lock, [pMutex] { return
!pMutex->m_FeedbackPipe.empty(); });
+ bWasEvent = pMutex->m_FeedbackPipe.front();
+ pMutex->m_FeedbackPipe.pop();
+ }
if (!bWasEvent && bWait)
{
// block & release YieldMutex until the main thread does something
diff --git a/vcl/inc/headless/svpinst.hxx b/vcl/inc/headless/svpinst.hxx
index e9aada5bc001..efe32761f5c1 100644
--- a/vcl/inc/headless/svpinst.hxx
+++ b/vcl/inc/headless/svpinst.hxx
@@ -29,6 +29,8 @@
#include <unx/genprn.h>
#include <condition_variable>
+#include <mutex>
+#include <queue>
#include <sys/time.h>
@@ -66,7 +68,9 @@ private:
// at least one subclass of SvpSalInstance (GTK3) that doesn't use them.
friend class SvpSalInstance;
// members for communication from main thread to non-main thread
- int m_FeedbackFDs[2];
+ std::mutex m_FeedbackMutex;
+ std::queue<bool> m_FeedbackPipe;
+ std::condition_variable m_FeedbackCV;
osl::Condition m_NonMainWaitingYieldCond;
// members for communication from non-main thread to main thread
bool m_bNoYieldLock = false; // accessed only on main
thread
@@ -104,8 +108,7 @@ public:
SvpSalInstance( std::unique_ptr<SalYieldMutex> pMutex );
virtual ~SvpSalInstance() override;
- void CloseWakeupPipe(bool log);
- void CreateWakeupPipe(bool log);
+ void CloseWakeupPipe();
void Wakeup(SvpRequest request = SvpRequest::NONE);
void StartTimer( sal_uInt64 nMS );
