Skip to content

Commit 91526a2

Browse files
author
Tor Lillqvist
committed
Use std synchronisation APIs instead of a pipe
The immediate reason for this is that pipes are broken in the Emscripten runtime, see emscripten-core/emscripten#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 <[email protected]> Tested-by: Jenkins
1 parent 777df29 commit 91526a2

File tree

2 files changed

+22
-69
lines changed

2 files changed

+22
-69
lines changed

vcl/headless/svpinst.cxx

Lines changed: 16 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,6 @@
2222

2323
#include <mutex>
2424

25-
#include <unistd.h>
26-
#include <errno.h>
27-
#include <fcntl.h>
2825
#include <pthread.h>
2926
#include <sys/time.h>
3027
#include <poll.h>
@@ -76,14 +73,13 @@ do { \
7673
#define DBG_TESTSVPYIELDMUTEX() ((void)0)
7774
#endif
7875

79-
#if !defined(ANDROID) && !defined(IOS)
76+
#if !defined(ANDROID) && !defined(IOS) && !defined(EMSCRIPTEN)
8077

8178
static void atfork_child()
8279
{
8380
if (SvpSalInstance::s_pDefaultInstance != nullptr)
8481
{
85-
SvpSalInstance::s_pDefaultInstance->CloseWakeupPipe(false);
86-
SvpSalInstance::s_pDefaultInstance->CreateWakeupPipe(false);
82+
SvpSalInstance::s_pDefaultInstance->CloseWakeupPipe();
8783
}
8884
}
8985

@@ -97,10 +93,9 @@ SvpSalInstance::SvpSalInstance( std::unique_ptr<SalYieldMutex> pMutex )
9793
m_nTimeoutMS = 0;
9894

9995
m_MainThread = osl::Thread::getCurrentIdentifier();
100-
CreateWakeupPipe(true);
10196
if( s_pDefaultInstance == nullptr )
10297
s_pDefaultInstance = this;
103-
#if !defined(ANDROID) && !defined(IOS)
98+
#if !defined(ANDROID) && !defined(IOS) && !defined(EMSCRIPTEN)
10499
pthread_atfork(nullptr, nullptr, atfork_child);
105100
#endif
106101
}
@@ -109,62 +104,16 @@ SvpSalInstance::~SvpSalInstance()
109104
{
110105
if( s_pDefaultInstance == this )
111106
s_pDefaultInstance = nullptr;
112-
CloseWakeupPipe(true);
107+
CloseWakeupPipe();
113108
}
114109

115-
void SvpSalInstance::CloseWakeupPipe(bool log)
110+
void SvpSalInstance::CloseWakeupPipe()
116111
{
117112
SvpSalYieldMutex *const pMutex(dynamic_cast<SvpSalYieldMutex*>(GetYieldMutex()));
118113
if (!pMutex)
119114
return;
120-
if (pMutex->m_FeedbackFDs[0] != -1)
121-
{
122-
if (log)
123-
{
124-
SAL_INFO("vcl.headless", "CloseWakeupPipe: Closing inherited feedback pipe: [" << pMutex->m_FeedbackFDs[0] << "," << pMutex->m_FeedbackFDs[1] << "]");
125-
}
126-
close (pMutex->m_FeedbackFDs[0]);
127-
close (pMutex->m_FeedbackFDs[1]);
128-
pMutex->m_FeedbackFDs[0] = pMutex->m_FeedbackFDs[1] = -1;
129-
}
130-
}
131-
132-
void SvpSalInstance::CreateWakeupPipe(bool log)
133-
{
134-
SvpSalYieldMutex *const pMutex(dynamic_cast<SvpSalYieldMutex*>(GetYieldMutex()));
135-
if (!pMutex)
136-
return;
137-
if (pipe (pMutex->m_FeedbackFDs) == -1)
138-
{
139-
if (log)
140-
{
141-
SAL_WARN("vcl.headless", "Could not create feedback pipe: " << strerror(errno));
142-
std::abort();
143-
}
144-
}
145-
else
146-
{
147-
if (log)
148-
{
149-
SAL_INFO("vcl.headless", "CreateWakeupPipe: Created feedback pipe: [" << pMutex->m_FeedbackFDs[0] << "," << pMutex->m_FeedbackFDs[1] << "]");
150-
}
151-
152-
int flags;
153-
154-
// set close-on-exec descriptor flag.
155-
if ((flags = fcntl (pMutex->m_FeedbackFDs[0], F_GETFD)) != -1)
156-
{
157-
flags |= FD_CLOEXEC;
158-
(void) fcntl(pMutex->m_FeedbackFDs[0], F_SETFD, flags);
159-
}
160-
if ((flags = fcntl (pMutex->m_FeedbackFDs[1], F_GETFD)) != -1)
161-
{
162-
flags |= FD_CLOEXEC;
163-
(void) fcntl(pMutex->m_FeedbackFDs[1], F_SETFD, flags);
164-
}
165-
166-
// retain the default blocking I/O for feedback pipe
167-
}
115+
while (!pMutex->m_FeedbackPipe.empty())
116+
pMutex->m_FeedbackPipe.pop();
168117
}
169118

170119
void SvpSalInstance::TriggerUserEventProcessing()
@@ -328,9 +277,6 @@ void SvpSalInstance::ProcessEvent( SalUserEvent aEvent )
328277

329278
SvpSalYieldMutex::SvpSalYieldMutex()
330279
{
331-
#ifndef IOS
332-
m_FeedbackFDs[0] = m_FeedbackFDs[1] = -1;
333-
#endif
334280
}
335281

336282
SvpSalYieldMutex::~SvpSalYieldMutex()
@@ -367,11 +313,11 @@ void SvpSalYieldMutex::doAcquire(sal_uInt32 const nLockCount)
367313
m_bNoYieldLock = true;
368314
bool const bEvents = pInst->DoYield(false, request == SvpRequest::MainThreadDispatchAllEvents);
369315
m_bNoYieldLock = false;
370-
if (write(m_FeedbackFDs[1], &bEvents, sizeof(bool)) != sizeof(bool))
371316
{
372-
SAL_WARN("vcl.headless", "Could not write: " << strerror(errno));
373-
std::abort();
317+
std::lock_guard lock(m_FeedbackMutex);
318+
m_FeedbackPipe.push(bEvents);
374319
}
320+
m_FeedbackCV.notify_all();
375321
}
376322
}
377323
while (true);
@@ -534,8 +480,12 @@ bool SvpSalInstance::DoYield(bool bWait, bool bHandleAllCurrentEvents)
534480
: SvpRequest::MainThreadDispatchOneEvent);
535481

536482
// blocking read (for synchronisation)
537-
auto const nRet = read(pMutex->m_FeedbackFDs[0], &bWasEvent, sizeof(bool));
538-
assert(nRet == 1); (void) nRet;
483+
{
484+
std::unique_lock lock(pMutex->m_FeedbackMutex);
485+
pMutex->m_FeedbackCV.wait(lock, [pMutex] { return !pMutex->m_FeedbackPipe.empty(); });
486+
bWasEvent = pMutex->m_FeedbackPipe.front();
487+
pMutex->m_FeedbackPipe.pop();
488+
}
539489
if (!bWasEvent && bWait)
540490
{
541491
// block & release YieldMutex until the main thread does something

vcl/inc/headless/svpinst.hxx

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@
2929
#include <unx/genprn.h>
3030

3131
#include <condition_variable>
32+
#include <mutex>
33+
#include <queue>
3234

3335
#include <sys/time.h>
3436

@@ -66,7 +68,9 @@ private:
6668
// at least one subclass of SvpSalInstance (GTK3) that doesn't use them.
6769
friend class SvpSalInstance;
6870
// members for communication from main thread to non-main thread
69-
int m_FeedbackFDs[2];
71+
std::mutex m_FeedbackMutex;
72+
std::queue<bool> m_FeedbackPipe;
73+
std::condition_variable m_FeedbackCV;
7074
osl::Condition m_NonMainWaitingYieldCond;
7175
// members for communication from non-main thread to main thread
7276
bool m_bNoYieldLock = false; // accessed only on main thread
@@ -104,8 +108,7 @@ public:
104108
SvpSalInstance( std::unique_ptr<SalYieldMutex> pMutex );
105109
virtual ~SvpSalInstance() override;
106110

107-
void CloseWakeupPipe(bool log);
108-
void CreateWakeupPipe(bool log);
111+
void CloseWakeupPipe();
109112
void Wakeup(SvpRequest request = SvpRequest::NONE);
110113

111114
void StartTimer( sal_uInt64 nMS );

0 commit comments

Comments
 (0)