setup_native/source/win32/customactions/inst_msu/inst_msu.cxx |   92 +++++++---
 1 file changed, 73 insertions(+), 19 deletions(-)

New commits:
commit e8c9b7bf8325961b8123cf28cb32b902ed11860b
Author:     Mike Kaganski <mike.kagan...@collabora.com>
AuthorDate: Thu Mar 21 00:32:43 2019 +0300
Commit:     Caolán McNamara <caol...@redhat.com>
CommitDate: Mon Mar 25 16:35:43 2019 +0100

    tdf#123832: Try to prevent/control hung wusa.exe while installing UCRT
    
    Reportedly under some circumstances execution of wusa.exe may hang
    for very long time (available logs show more than 90 min). LO MSI
    installer waits for wusa.exe indefinitely in its deferred inst_msu
    custom action; during this wait, one can't abort the installation.
    
    There is an evidence [1] that stopping WU service prior to wusa
    launch might prevent this hang.
    
    The patch does two things:
    1. It stops running WU service prior to wusa launch.
    2. It adds a check for user input by executing MsiProcessMessage
    twice a second, and checking its return value, to allow early
    return in case user cancels the installation.
    
    This is a blind shot, since I cannot reproduce the problem myself.
    
    [1] https://superuser.com/questions/1044528/
    
    Change-Id: I8bf4ce52b3e9d98a4f90af3abcc49ac63b6a0e40
    Reviewed-on: https://gerrit.libreoffice.org/69487
    Reviewed-by: Mike Kaganski <mike.kagan...@collabora.com>
    Tested-by: Mike Kaganski <mike.kagan...@collabora.com>
    Reviewed-on: https://gerrit.libreoffice.org/69572
    Tested-by: Jenkins
    Reviewed-by: Caolán McNamara <caol...@redhat.com>
    Tested-by: Caolán McNamara <caol...@redhat.com>

diff --git a/setup_native/source/win32/customactions/inst_msu/inst_msu.cxx 
b/setup_native/source/win32/customactions/inst_msu/inst_msu.cxx
index 96fb88f4b889..d81532020cce 100644
--- a/setup_native/source/win32/customactions/inst_msu/inst_msu.cxx
+++ b/setup_native/source/win32/customactions/inst_msu/inst_msu.cxx
@@ -35,6 +35,14 @@ template <typename IntType> std::string Num2Dec(IntType n)
     return sMsg.str();
 }
 
+std::string Win32ErrorMessage(const char* sFunc, DWORD nWin32Error)
+{
+    std::stringstream sMsg;
+    sMsg << sFunc << " failed with Win32 error code " << Num2Hex(nWin32Error) 
<< "!";
+
+    return sMsg.str();
+}
+
 void ThrowHResult(const char* sFunc, HRESULT hr)
 {
     std::stringstream sMsg;
@@ -51,10 +59,7 @@ void CheckHResult(const char* sFunc, HRESULT hr)
 
 void ThrowWin32Error(const char* sFunc, DWORD nWin32Error)
 {
-    std::stringstream sMsg;
-    sMsg << sFunc << " failed with Win32 error code " << Num2Hex(nWin32Error) 
<< "!";
-
-    throw std::exception(sMsg.str().c_str());
+    throw std::exception(Win32ErrorMessage(sFunc, nWin32Error).c_str());
 }
 
 void ThrowLastError(const char* sFunc) { ThrowWin32Error(sFunc, 
GetLastError()); }
@@ -177,6 +182,8 @@ public:
 };
 
 // Checks if Windows Update service is disabled, and if it is, enables it 
temporarily.
+// Also stops the service if it's currently running, because it seems that 
wusa.exe
+// does not freeze when it starts the service itself.
 class WUServiceEnabler
 {
 public:
@@ -219,7 +226,14 @@ private:
                 ThrowLastError("OpenServiceW");
             WriteLog(hInstall, "Obtained WU service handle");
 
-            if (ServiceStatus(hInstall, hService.get()) == SERVICE_RUNNING
+            const DWORD nCurrentStatus = ServiceStatus(hInstall, 
hService.get());
+            // Stop currently running service to prevent wusa.exe from hanging 
trying to detect if the
+            // update is applicable (sometimes this freezes it ~indefinitely; 
it seems that it doesn't
+            // happen if wusa.exe starts the service itself: 
https://superuser.com/questions/1044528/).
+            if (nCurrentStatus == SERVICE_RUNNING)
+                StopService(hInstall, hService.get());
+
+            if (nCurrentStatus == SERVICE_RUNNING
                 || !EnsureServiceEnabled(hInstall, hService.get(), true))
             {
                 // No need to restore anything back, since we didn't change 
config
@@ -324,17 +338,26 @@ private:
 
     static void StopService(MSIHANDLE hInstall, SC_HANDLE hService)
     {
-        if (ServiceStatus(hInstall, hService) != SERVICE_STOPPED)
+        try
         {
-            SERVICE_STATUS aServiceStatus{};
-            if (!ControlService(hService, SERVICE_CONTROL_STOP, 
&aServiceStatus))
-                ThrowLastError("ControlService");
-            WriteLog(hInstall,
-                     "Successfully sent SERVICE_CONTROL_STOP code to Windows 
Update service");
-            // No need to wait for the service stopped
+            if (ServiceStatus(hInstall, hService) != SERVICE_STOPPED)
+            {
+                SERVICE_STATUS aServiceStatus{};
+                if (!ControlService(hService, SERVICE_CONTROL_STOP, 
&aServiceStatus))
+                    WriteLog(hInstall, Win32ErrorMessage("ControlService", 
GetLastError()));
+                else
+                    WriteLog(
+                        hInstall,
+                        "Successfully sent SERVICE_CONTROL_STOP code to 
Windows Update service");
+                // No need to wait for the service stopped
+            }
+            else
+                WriteLog(hInstall, "Windows Update service is not running");
+        }
+        catch (std::exception& e)
+        {
+            WriteLog(hInstall, e.what());
         }
-        else
-            WriteLog(hInstall, "Windows Update service is not running");
     }
 
     MSIHANDLE mhInstall;
@@ -446,7 +469,8 @@ extern "C" __declspec(dllexport) UINT __stdcall 
InstallMSU(MSIHANDLE hInstall)
         WriteLog(hInstall, "Got CustomActionData value:", sBinaryName);
         auto aDeleteFileGuard(Guard(sBinaryName));
 
-        // In case the Windows Update service is disabled, we temporarily 
enable it here
+        // In case the Windows Update service is disabled, we temporarily 
enable it here. We also
+        // stop running WU service, to avoid wusa.exe freeze (see comment in 
EnableWUService).
         WUServiceEnabler aWUServiceEnabler(hInstall);
 
         const bool bWow64Process = IsWow64Process();
@@ -466,12 +490,43 @@ extern "C" __declspec(dllexport) UINT __stdcall 
InstallMSU(MSIHANDLE hInstall)
         if (!CreateProcessW(sWUSAPath.c_str(), 
const_cast<LPWSTR>(sWUSACmd.c_str()), nullptr,
                             nullptr, FALSE, CREATE_NO_WINDOW, nullptr, 
nullptr, &si, &pi))
             ThrowLastError("CreateProcessW");
+        CloseHandle(pi.hThread);
         auto aCloseProcHandleGuard(Guard(pi.hProcess));
         WriteLog(hInstall, "CreateProcessW succeeded");
 
-        DWORD nWaitResult = WaitForSingleObject(pi.hProcess, INFINITE);
-        if (nWaitResult != WAIT_OBJECT_0)
-            ThrowWin32Error("WaitForSingleObject", nWaitResult);
+        {
+            // This block waits when the started wusa.exe process finishes. 
Since it's possible
+            // for wusa.exe in some circumstances to wait really long 
(indefinitely?), we use
+            // MsiProcessMessage to check for user input: it returns IDCANCEL 
when user cancels
+            // installation.
+            PMSIHANDLE hProgressRec = MsiCreateRecord(3);
+            // Use explicit progress messages
+            MsiRecordSetInteger(hProgressRec, 1, 1);
+            MsiRecordSetInteger(hProgressRec, 2, 1);
+            MsiRecordSetInteger(hProgressRec, 3, 0);
+            int nResult = MsiProcessMessage(hInstall, INSTALLMESSAGE_PROGRESS, 
hProgressRec);
+            if (nResult == IDCANCEL)
+                return ERROR_INSTALL_USEREXIT;
+            // Prepare the record to following progress update calls
+            MsiRecordSetInteger(hProgressRec, 1, 2);
+            MsiRecordSetInteger(hProgressRec, 2, 0); // step by 0 - don't move 
progress
+            MsiRecordSetInteger(hProgressRec, 3, 0);
+            for (;;)
+            {
+                DWORD nWaitResult = WaitForSingleObject(pi.hProcess, 500);
+                if (nWaitResult == WAIT_OBJECT_0)
+                    break; // wusa.exe finished
+                else if (nWaitResult == WAIT_TIMEOUT)
+                {
+                    // Check if user has cancelled
+                    nResult = MsiProcessMessage(hInstall, 
INSTALLMESSAGE_PROGRESS, hProgressRec);
+                    if (nResult == IDCANCEL)
+                        return ERROR_INSTALL_USEREXIT;
+                }
+                else
+                    ThrowWin32Error("WaitForSingleObject", nWaitResult);
+            }
+        }
 
         DWORD nExitCode = 0;
         if (!GetExitCodeProcess(pi.hProcess, &nExitCode))
@@ -484,7 +539,6 @@ extern "C" __declspec(dllexport) UINT __stdcall 
InstallMSU(MSIHANDLE hInstall)
         switch (hr)
         {
             case S_OK:
-            case S_FALSE:
             case WU_S_ALREADY_INSTALLED:
             case WU_E_NOT_APPLICABLE: // Windows could lie us about its 
version, etc.
             case ERROR_SUCCESS_REBOOT_REQUIRED:
_______________________________________________
Libreoffice-commits mailing list
libreoffice-comm...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits

Reply via email to