Date: prev next · Thread: first prev next last
2013 Archives by date, by thread · List index


Hi,

I have submitted a patch for review:

    https://gerrit.libreoffice.org/1733

To pull it, you can do:

    git pull ssh://gerrit.libreoffice.org:29418/core refs/changes/33/1733/1

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@redhat.com>
(cherry picked from commit 0dce7eae55bf90d2a7171a1fb8663d66ba4ac6d3)
(cherry picked from commit 8376da60d5d272cf6b3ebee91934bbcd970c7658)
---
M desktop/source/app/officeipcthread.cxx
M desktop/source/app/officeipcthread.hxx
M desktop/unx/source/start.c
M desktop/win32/source/officeloader/officeloader.cxx
4 files changed, 89 insertions(+), 32 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 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 @@
         }
         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 @@
             // 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 @@
     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 @@
 
         if ( ( fd = connect_pipe( pPipePath ) ) >= 0 )
         {
-            rtl_uString *pCwdPath = NULL;
-            osl_getProcessWorkingDir( &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 );
 
-            bSentArgs = send_args( fd, 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 @@
 
                 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++ )
-                    {
-                        fSuccess = writeArgument(hPipe, ',', argv2[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 = WriteFile(  hPipe, "", 1, &dwBytesWritten, NULL );
+                        fSuccess = (dwBytesRead == (sizeof("InternalIPC::SendArguments") + 1) &&
+                            0 == strncmp( "InternalIPC::SendArguments", pBuffer, dwBytesRead - 1 ) 
);
+                    }
+                    if ( fSuccess )
+                    {
+                        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;

-- 
To view, visit https://gerrit.libreoffice.org/1733
To unsubscribe, visit https://gerrit.libreoffice.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I2e099a5804e1e8dd535cfd31ef454cffa44efa62
Gerrit-PatchSet: 1
Gerrit-Project: core
Gerrit-Branch: libreoffice-3-6
Gerrit-Owner: Stephan Bergmann <sbergman@redhat.com>
Gerrit-Reviewer: Pierre-Eric Pelloux-Prayer <pierre-eric@lanedo.com>


Context


Privacy Policy | Impressum (Legal Info) | Copyright information: Unless otherwise specified, all text and images on this website are licensed under the Creative Commons Attribution-Share Alike 3.0 License. This does not include the source code of LibreOffice, which is licensed under the Mozilla Public License (MPLv2). "LibreOffice" and "The Document Foundation" are registered trademarks of their corresponding registered owners or are in actual use as trademarks in one or more countries. Their respective logos and icons are also subject to international copyright laws. Use thereof is explained in our trademark policy.