[libreoffice/f18] Resolves: rhbz#885156 lockup when opening file over SMB
sbergmann
sbergmann at fedoraproject.org
Thu Jan 17 10:46:04 UTC 2013
commit 9d1e678d275ddaf1f37bb6102298709f9eab5ac2
Author: Stephan Bergmann <sbergman at redhat.com>
Date: Thu Jan 17 11:45:46 2013 +0100
Resolves: rhbz#885156 lockup when opening file over SMB
...re-reliable-startup-of-multiple-instances.patch | 220 ++++++++++++++++++++
...-33484-Terminate-OfficeIPCThread-by-closi.patch | 163 +++++++++++++++
libreoffice.spec | 23 ++-
3 files changed, 397 insertions(+), 9 deletions(-)
---
diff --git a/0001-startup-more-reliable-startup-of-multiple-instances.patch b/0001-startup-more-reliable-startup-of-multiple-instances.patch
new file mode 100644
index 0000000..ae80195
--- /dev/null
+++ b/0001-startup-more-reliable-startup-of-multiple-instances.patch
@@ -0,0 +1,220 @@
+From 8da863eec07ece1dc65e46e102fb8b047d0f96de Mon Sep 17 00:00:00 2001
+From: Pierre-Eric Pelloux-Prayer <pierre-eric at lanedo.com>
+Date: Tue, 20 Nov 2012 11:03:03 +0100
+Subject: [PATCH 1/2] startup: more reliable startup of multiple instances
+
+Until now, when a new soffice instance (S2) started and tried to
+connect to an existing soffice process (S1), S2 may have failed to
+boostrap due to race condition in communication over the shared pipe.
+
+S1 can be shutdown after S2 connected to it but _before_ S1 handled its
+arguments (code run after 'accept' method in OfficeIPCThread).
+This patch introduces a new message, sent by the main soffice after it
+has called accept if and only if it's not shutting down (see mbDowning
+ member).
+The other soffice waits for this message before enabling going in
+ PIPE_CONNECTED mode. If soffice fails to receive this message, pipe mode is
+left unchanged and after a quick pause, it will try again.
+
+Change-Id: I2e099a5804e1e8dd535cfd31ef454cffa44efa62
+Signed-off-by: Stephan Bergmann <sbergman at redhat.com>
+(cherry picked from commit 0dce7eae55bf90d2a7171a1fb8663d66ba4ac6d3)
+(cherry picked from commit 8376da60d5d272cf6b3ebee91934bbcd970c7658)
+---
+ desktop/source/app/officeipcthread.cxx | 40 ++++++++++++++-
+ desktop/source/app/officeipcthread.hxx | 2 +
+ desktop/unx/source/start.c | 17 ++++--
+ desktop/win32/source/officeloader/officeloader.cxx | 60 +++++++++++++---------
+ 4 files changed, 88 insertions(+), 31 deletions(-)
+
+diff --git a/desktop/source/app/officeipcthread.cxx b/desktop/source/app/officeipcthread.cxx
+index b9ede23..452a3b5 100644
+--- a/desktop/source/app/officeipcthread.cxx
++++ b/desktop/source/app/officeipcthread.cxx
+@@ -66,6 +66,8 @@ const char *OfficeIPCThread::sc_aShowSequence = "-tofront";
+ const int OfficeIPCThread::sc_nShSeqLength = 5;
+ const char *OfficeIPCThread::sc_aConfirmationSequence = "InternalIPC::ProcessingDone";
+ const int OfficeIPCThread::sc_nCSeqLength = 27;
++const char *OfficeIPCThread::sc_aSendArgumentsSequence = "InternalIPC::SendArguments";
++const int OfficeIPCThread::sc_nCSASeqLength = 26;
+
+ namespace { static char const ARGUMENT_PREFIX[] = "InternalIPC::Arguments"; }
+
+@@ -506,8 +508,31 @@ OfficeIPCThread::Status OfficeIPCThread::EnableOfficeIPCThread()
+ }
+ else if( pThread->maPipe.create( pThread->maPipeIdent.getStr(), osl_Pipe_OPEN, rSecurity )) // Creation not successfull, now we try to connect
+ {
+- // Pipe connected to first office
+- nPipeMode = PIPEMODE_CONNECTED;
++ osl::StreamPipe aStreamPipe(pThread->maPipe.getHandle());
++ char pReceiveBuffer[sc_nCSASeqLength + 1];
++ int nResult = 0;
++ int nBytes = 0;
++ int nBufSz = sc_nCSASeqLength + 1;
++ // read byte per byte
++ while ((nResult=aStreamPipe.recv( pReceiveBuffer+nBytes, nBufSz-nBytes))>0) {
++ nBytes += nResult;
++ if (pReceiveBuffer[nBytes-1]=='\0') {
++ break;
++ }
++ }
++ if (rtl::OString(sc_aSendArgumentsSequence).equals(pReceiveBuffer))
++ {
++ // Pipe connected to first office
++ nPipeMode = PIPEMODE_CONNECTED;
++ }
++ else
++ {
++ // Pipe connection failed (other office exited or crashed)
++ TimeValue tval;
++ tval.Seconds = 0;
++ tval.Nanosec = 500000000;
++ salhelper::Thread::wait( tval );
++ }
+ }
+ else
+ {
+@@ -661,6 +686,17 @@ void OfficeIPCThread::execute()
+ // down during wait
+ osl::ClearableMutexGuard aGuard( GetMutex() );
+
++ if (!mbDowning)
++ {
++ // notify client we're ready to process its args
++ int nBytes = 0;
++ int nResult = 0;
++ while (
++ (nResult = maStreamPipe.send(sc_aSendArgumentsSequence+nBytes, sc_nCSASeqLength-nBytes))>0 &&
++ ((nBytes += nResult) < sc_nCSASeqLength) ) ;
++ }
++ maStreamPipe.write("\0", 1);
++
+ // test byte by byte
+ const int nBufSz = 2048;
+ char pBuf[nBufSz];
+diff --git a/desktop/source/app/officeipcthread.hxx b/desktop/source/app/officeipcthread.hxx
+index f60a134..ba40b57 100644
+--- a/desktop/source/app/officeipcthread.hxx
++++ b/desktop/source/app/officeipcthread.hxx
+@@ -103,6 +103,8 @@ class OfficeIPCThread : public salhelper::Thread
+ static const int sc_nShSeqLength;
+ static const char *sc_aConfirmationSequence;
+ static const int sc_nCSeqLength;
++ static const char *sc_aSendArgumentsSequence;
++ static const int sc_nCSASeqLength;
+
+ OfficeIPCThread();
+
+diff --git a/desktop/unx/source/start.c b/desktop/unx/source/start.c
+index c6a8c2b..f7a2306 100644
+--- a/desktop/unx/source/start.c
++++ b/desktop/unx/source/start.c
+@@ -837,10 +837,19 @@ SAL_IMPLEMENT_MAIN_WITH_ARGS( argc, argv )
+
+ if ( ( fd = connect_pipe( pPipePath ) ) >= 0 )
+ {
+- rtl_uString *pCwdPath = NULL;
+- osl_getProcessWorkingDir( &pCwdPath );
+-
+- bSentArgs = send_args( fd, pCwdPath );
++ // Wait for answer
++ char resp[ strlen( "InternalIPC::SendArguments" ) + 1];
++ ssize_t n = read( fd, resp, SAL_N_ELEMENTS( resp ) );
++ if (n == (ssize_t) SAL_N_ELEMENTS( resp )
++ && (memcmp(
++ resp, "InternalIPC::SendArguments",
++ SAL_N_ELEMENTS( resp ) - 1) == 0)) {
++ rtl_uString *pCwdPath = NULL;
++ osl_getProcessWorkingDir( &pCwdPath );
++
++ // Then send args
++ bSentArgs = send_args( fd, pCwdPath );
++ }
+
+ close( fd );
+ }
+diff --git a/desktop/win32/source/officeloader/officeloader.cxx b/desktop/win32/source/officeloader/officeloader.cxx
+index 14c2303..32ada0b 100644
+--- a/desktop/win32/source/officeloader/officeloader.cxx
++++ b/desktop/win32/source/officeloader/officeloader.cxx
+@@ -273,42 +273,52 @@ int WINAPI _tWinMain( HINSTANCE, HINSTANCE, LPTSTR, int )
+
+ if ( INVALID_HANDLE_VALUE != hPipe )
+ {
+- DWORD dwBytesWritten;
+- int argc2 = 0;
+- LPWSTR *argv2 = CommandLineToArgvW( GetCommandLine(), &argc2 );
+-
+- fSuccess = WriteFile( hPipe, RTL_CONSTASCII_STRINGPARAM("InternalIPC::Arguments"), &dwBytesWritten, NULL );
+- if (fSuccess) {
+- if (cwdLen > 0) {
+- fSuccess = writeArgument(hPipe, '2', cwd);
+- } else {
+- fSuccess = WriteFile(
+- hPipe, RTL_CONSTASCII_STRINGPARAM("0"),
+- &dwBytesWritten, NULL);
+- }
+- }
+- for ( int argn = 1; fSuccess && argn < argc2; argn++ )
++ DWORD dwBytesRead = 0;
++ char *pBuffer = (char *)_alloca( sizeof("InternalIPC::SendArguments") + 1);
++ fSuccess = ReadFile( hPipe, pBuffer, sizeof("InternalIPC::SendArguments") + 1, &dwBytesRead, NULL );
++ if ( fSuccess )
+ {
+- fSuccess = writeArgument(hPipe, ',', argv2[argn]);
++ fSuccess = (dwBytesRead == (sizeof("InternalIPC::SendArguments") + 1) &&
++ 0 == strncmp( "InternalIPC::SendArguments", pBuffer, dwBytesRead - 1 ) );
+ }
+-
+ if ( fSuccess )
+ {
+- fSuccess = WriteFile( hPipe, "", 1, &dwBytesWritten, NULL );
++ DWORD dwBytesWritten;
++ int argc2 = 0;
++ LPWSTR *argv2 = CommandLineToArgvW( GetCommandLine(), &argc2 );
++
++ fSuccess = WriteFile( hPipe, RTL_CONSTASCII_STRINGPARAM("InternalIPC::Arguments"), &dwBytesWritten, NULL );
++ if (fSuccess) {
++ if (cwdLen > 0) {
++ fSuccess = writeArgument(hPipe, '2', cwd);
++ } else {
++ fSuccess = WriteFile(
++ hPipe, RTL_CONSTASCII_STRINGPARAM("0"),
++ &dwBytesWritten, NULL);
++ }
++ }
++ for ( int argn = 1; fSuccess && argn < argc2; argn++ )
++ {
++ fSuccess = writeArgument(hPipe, ',', argv2[argn]);
++ }
++
+ if ( fSuccess )
+ {
+- DWORD dwBytesRead = 0;
+- char *pBuffer = (char *)_alloca( sizeof(PIPE_TERMINATION_SEQUENCE) );
+- fSuccess = ReadFile( hPipe, pBuffer, sizeof(PIPE_TERMINATION_SEQUENCE) - 1, &dwBytesRead, NULL );
++ fSuccess = WriteFile( hPipe, "", 1, &dwBytesWritten, NULL );
+ if ( fSuccess )
+ {
+- pBuffer[dwBytesRead] = 0;
+- if ( 0 != strcmp( PIPE_TERMINATION_SEQUENCE, pBuffer ) )
+- fSuccess = FALSE;
++ DWORD dwBytesRead = 0;
++ char *pBuffer = (char *)_alloca( sizeof(PIPE_TERMINATION_SEQUENCE) );
++ fSuccess = ReadFile( hPipe, pBuffer, sizeof(PIPE_TERMINATION_SEQUENCE) - 1, &dwBytesRead, NULL );
++ if ( fSuccess )
++ {
++ pBuffer[dwBytesRead] = 0;
++ if ( 0 != strcmp( PIPE_TERMINATION_SEQUENCE, pBuffer ) )
++ fSuccess = FALSE;
++ }
+ }
+ }
+ }
+-
+ CloseHandle( hPipe );
+
+ return fSuccess ? 0 : -1;
+--
+1.7.11.7
+
diff --git a/0002-Related-fdo-33484-Terminate-OfficeIPCThread-by-closi.patch b/0002-Related-fdo-33484-Terminate-OfficeIPCThread-by-closi.patch
new file mode 100644
index 0000000..c3e5585
--- /dev/null
+++ b/0002-Related-fdo-33484-Terminate-OfficeIPCThread-by-closi.patch
@@ -0,0 +1,163 @@
+From 5b29409ae1ec8c75296a257dfc02b6acb296a527 Mon Sep 17 00:00:00 2001
+From: Stephan Bergmann <sbergman at redhat.com>
+Date: Thu, 13 Dec 2012 15:41:10 +0100
+Subject: [PATCH 2/2] Related fdo#33484: Terminate OfficeIPCThread by closing
+ the accepting pipe
+
+... (and setting mbDowning to indicate an error returned from accept() is due to
+termination) instead of setting up an extra pipe connection to send an
+"InternalIPC::TerminateThread" message (which allegedly caused deadlocks, see
+<https://gerrit.libreoffice.org/#/c/1311/> "Change Idf933915: office ipc: use
+timeout pipe feature when connecting to self").
+
+Change-Id: Id302ca13112fc409685e7665b38f1030704a0ccf
+(cherry picked from commit 4ce2602befd59e69264d8e4ced8730b40c2b947c)
+
+Conflicts:
+ desktop/source/app/officeipcthread.cxx
+ desktop/source/app/officeipcthread.hxx
+(cherry picked from commit 6527b8a135c20e223a6fcf7c49835205a99ff02a)
+---
+ desktop/source/app/officeipcthread.cxx | 54 +++++++++++++++-------------------
+ desktop/source/app/officeipcthread.hxx | 1 -
+ 2 files changed, 23 insertions(+), 32 deletions(-)
+
+diff --git a/desktop/source/app/officeipcthread.cxx b/desktop/source/app/officeipcthread.cxx
+index 452a3b5..1d37ee0 100644
+--- a/desktop/source/app/officeipcthread.cxx
++++ b/desktop/source/app/officeipcthread.cxx
+@@ -60,8 +60,6 @@ using ::rtl::OString;
+ using ::rtl::OUString;
+ using ::rtl::OUStringBuffer;
+
+-const char *OfficeIPCThread::sc_aTerminationSequence = "InternalIPC::TerminateThread";
+-const int OfficeIPCThread::sc_nTSeqLength = 28;
+ const char *OfficeIPCThread::sc_aShowSequence = "-tofront";
+ const int OfficeIPCThread::sc_nShSeqLength = 5;
+ const char *OfficeIPCThread::sc_aConfirmationSequence = "InternalIPC::ProcessingDone";
+@@ -436,8 +434,6 @@ OfficeIPCThread::Status OfficeIPCThread::EnableOfficeIPCThread()
+
+ rtl::Reference< OfficeIPCThread > pThread(new OfficeIPCThread);
+
+- pThread->maPipeIdent = OUString( "SingleOfficeIPC_" );
+-
+ // The name of the named pipe is created with the hashcode of the user installation directory (without /user). We have to retrieve
+ // this information from a unotools implementation.
+ ::utl::Bootstrap::PathStatus aLocateResult = ::utl::Bootstrap::locateUserInstallation( aUserInstallPath );
+@@ -494,19 +490,19 @@ OfficeIPCThread::Status OfficeIPCThread::EnableOfficeIPCThread()
+ if ( aUserInstallPathHashCode.isEmpty() )
+ return IPC_STATUS_BOOTSTRAP_ERROR; // Something completely broken, we cannot create a valid hash code!
+
+- pThread->maPipeIdent = pThread->maPipeIdent + aUserInstallPathHashCode;
++ OUString aPipeIdent( "SingleOfficeIPC_" + aUserInstallPathHashCode );
+
+ PipeMode nPipeMode = PIPEMODE_DONTKNOW;
+ do
+ {
+ osl::Security &rSecurity = Security::get();
+ // Try to create pipe
+- if ( pThread->maPipe.create( pThread->maPipeIdent.getStr(), osl_Pipe_CREATE, rSecurity ))
++ if ( pThread->maPipe.create( aPipeIdent.getStr(), osl_Pipe_CREATE, rSecurity ))
+ {
+ // Pipe created
+ nPipeMode = PIPEMODE_CREATED;
+ }
+- else if( pThread->maPipe.create( pThread->maPipeIdent.getStr(), osl_Pipe_OPEN, rSecurity )) // Creation not successfull, now we try to connect
++ else if( pThread->maPipe.create( aPipeIdent.getStr(), osl_Pipe_OPEN, rSecurity )) // Creation not successfull, now we try to connect
+ {
+ osl::StreamPipe aStreamPipe(pThread->maPipe.getHandle());
+ char pReceiveBuffer[sc_nCSASeqLength + 1];
+@@ -610,18 +606,8 @@ void OfficeIPCThread::DisableOfficeIPCThread()
+ pGlobalOfficeIPCThread);
+ pGlobalOfficeIPCThread.clear();
+
+- // send thread a termination message
+- // this is done so the subsequent join will not hang
+- // because the thread hangs in accept of pipe
+- osl::StreamPipe aPipe ( pOfficeIPCThread->maPipeIdent, osl_Pipe_OPEN, Security::get() );
+- if (aPipe.is())
+- {
+- aPipe.send( sc_aTerminationSequence, sc_nTSeqLength+1 ); // also send 0-byte
+-
+- // close the pipe so that the streampipe on the other
+- // side produces EOF
+- aPipe.close();
+- }
++ pOfficeIPCThread->mbDowning = true;
++ pOfficeIPCThread->maPipe.close();
+
+ // release mutex to avoid deadlocks
+ aMutex.clear();
+@@ -686,22 +672,23 @@ void OfficeIPCThread::execute()
+ // down during wait
+ osl::ClearableMutexGuard aGuard( GetMutex() );
+
+- if (!mbDowning)
++ if ( mbDowning )
+ {
+- // notify client we're ready to process its args
+- int nBytes = 0;
+- int nResult = 0;
+- while (
+- (nResult = maStreamPipe.send(sc_aSendArgumentsSequence+nBytes, sc_nCSASeqLength-nBytes))>0 &&
+- ((nBytes += nResult) < sc_nCSASeqLength) ) ;
++ break;
+ }
++
++ // notify client we're ready to process its args
++ int nBytes = 0;
++ int nResult;
++ while (
++ (nResult = maStreamPipe.send(sc_aSendArgumentsSequence+nBytes, sc_nCSASeqLength-nBytes))>0 &&
++ ((nBytes += nResult) < sc_nCSASeqLength) ) ;
+ maStreamPipe.write("\0", 1);
+
+ // test byte by byte
+ const int nBufSz = 2048;
+ char pBuf[nBufSz];
+- int nBytes = 0;
+- int nResult = 0;
++ nBytes = 0;
+ rtl::OStringBuffer aBuf;
+ // read into pBuf until '\0' is read or read-error
+ while ((nResult=maStreamPipe.recv( pBuf+nBytes, nBufSz-nBytes))>0) {
+@@ -719,9 +706,6 @@ void OfficeIPCThread::execute()
+ if (aArguments.isEmpty())
+ continue;
+
+- // is this a termination message ? if so, terminate
+- if (aArguments.equalsL(sc_aTerminationSequence, sc_nTSeqLength) || mbDowning)
+- return;
+ std::auto_ptr< CommandLineArgs > aCmdLineArgs;
+ try
+ {
+@@ -938,6 +922,14 @@ void OfficeIPCThread::execute()
+ }
+ else
+ {
++ {
++ osl::MutexGuard aGuard( GetMutex() );
++ if ( mbDowning )
++ {
++ break;
++ }
++ }
++
+ #if (OSL_DEBUG_LEVEL > 1) || defined DBG_UTIL
+ fprintf( stderr, "Error on accept: %d\n", (int)nError );
+ #endif
+diff --git a/desktop/source/app/officeipcthread.hxx b/desktop/source/app/officeipcthread.hxx
+index ba40b57..09e56da 100644
+--- a/desktop/source/app/officeipcthread.hxx
++++ b/desktop/source/app/officeipcthread.hxx
+@@ -82,7 +82,6 @@ class OfficeIPCThread : public salhelper::Thread
+
+ osl::Pipe maPipe;
+ osl::StreamPipe maStreamPipe;
+- rtl::OUString maPipeIdent;
+ bool mbDowning;
+ bool mbRequestsEnabled;
+ int mnPendingRequests;
+--
+1.7.11.7
+
diff --git a/libreoffice.spec b/libreoffice.spec
index 3c557c6..8108eaa 100644
--- a/libreoffice.spec
+++ b/libreoffice.spec
@@ -245,11 +245,13 @@ Patch25: 0001-Resolves-fdo-56198-collect-scrollbar-click-preferenc.patch
Patch26: 0001-bigendian-utext-mixup-triggering-regression-test-fai.patch
Patch27: 0001-fiddle-system-db-test-to-link-on-RHEL-6.patch
Patch28: 0001-split-qnametostr-up-to-try-and-make-.o-s-small-enoug.patch
+Patch29: 0001-startup-more-reliable-startup-of-multiple-instances.patch
+Patch30: 0002-Related-fdo-33484-Terminate-OfficeIPCThread-by-closi.patch
%if 0%{?rhel} && 0%{?rhel} < 7
-Patch29: libreoffice-rhel6gcj.patch
-Patch30: libreoffice-rhel6poppler.patch
-Patch31: libreoffice-rhel6langs.patch
-Patch32: 0001-rhbz-891082-CMXDocument-isSupported-catch-exceptions.patch
+Patch31: libreoffice-rhel6gcj.patch
+Patch32: libreoffice-rhel6poppler.patch
+Patch33: libreoffice-rhel6langs.patch
+Patch34: 0001-rhbz-891082-CMXDocument-isSupported-catch-exceptions.patch
%endif
%{!?python_sitearch: %global python_sitearch %(%{__python} -c "from distutils.sysconfig import get_python_lib; print(get_python_lib(1))")}
@@ -989,11 +991,13 @@ mv -f redhat.soc extras/source/palettes/standard.soc
%patch26 -p1 -b .bigendian-utext-mixup-triggering-regression-test-fai.patch
%patch27 -p1 -b .fiddle-system-db-test-to-link-on-RHEL-6.patch
%patch28 -p1 -b .split-qnametostr-up-to-try-and-make-.o-s-small-enoug.patch
+%patch29 -p1 -b .startup-more-reliable-startup-of-multiple-instances.patch
+%patch30 -p1 -b .Related-fdo-33484-Terminate-OfficeIPCThread-by-closi.patch
%if 0%{?rhel} && 0%{?rhel} < 7
-%patch29 -p1 -b .rhel6gcj.patch
-%patch30 -p1 -b .rhel6poppler.patch
-%patch31 -p1 -b .rhel6langs.patch
-%patch32 -p1 -b .rhbz-891082-CMXDocument-isSupported-catch-exceptions.patch
+%patch31 -p1 -b .rhel6gcj.patch
+%patch32 -p1 -b .rhel6poppler.patch
+%patch33 -p1 -b .rhel6langs.patch
+%patch34 -p1 -b .rhbz-891082-CMXDocument-isSupported-catch-exceptions.patch
%endif
# TODO: check this
@@ -2262,8 +2266,9 @@ update-desktop-database %{_datadir}/applications &> /dev/null || :
%endif
%changelog
-* Sat Jan 05 2013 Michael Stahl <mstahl at redhat.com> - 1:3.6.4.3-2-UNBUILT
+* Sat Jan 05 2013 Michael Stahl <mstahl at redhat.com> - 1:3.6.4.3-2
- Resolves: rhbz#891082 catch libcdr exceptions
+- Resolves: rhbz#885156 lockup when opening file over SMB
* Thu Nov 29 2012 David Tardon <dtardon at redhat.com> - 1:3.6.4.3-1
- 3.6.4 rc3
More information about the scm-commits
mailing list