Merge branch 'fix/SDK-4573_Replace_GetOverlappedResultEx_to_support_W…

…indows_7' into 'release/v8.0.0'

SDK-4573 Replace GetOverlappedResultEx to support Windows 7 (v8.0.0)

See merge request sdk/sdk!6035

include/mega/win32/gfx/worker/comms.h

@@ -63,11 +63,17 @@ class WinOverlapped final
 
     OVERLAPPED* data();
 
-    bool isValid() const { return mOverlapped.hEvent != NULL; };
+    bool isValid() const
+    {
+        return mOverlapped.hEvent != NULL;
+    };
+
+    // Return an error code and error string on error
+    std::pair<std::error_code, std::string> waitForCompletion(DWORD mWaitMs);
 
 private:
     OVERLAPPED mOverlapped;
 };
 
 } // namespace
-}
+}

src/gfx/worker/client.cpp

@@ -36,7 +36,6 @@ bool GfxClient::runHello(const std::string& text)
     auto response = sendAndReceive<CommandHelloResponse>(endpoint.get(), command);
     if (response)
     {
-        LOG_verbose << "GfxClient gets hello response: " << response->Text;
         return true;
     }
     else

src/win32/gfx/worker/comms.cpp

@@ -24,11 +24,10 @@ WinOverlapped::WinOverlapped()
 {
     mOverlapped.Offset = 0;
     mOverlapped.OffsetHigh = 0;
-    mOverlapped.hEvent = CreateEvent(
-        NULL,    // default security attribute
-        TRUE,    // manual-reset event
-        TRUE,    // initial state = signaled
-        NULL);   // unnamed event object
+    mOverlapped.hEvent = CreateEvent(NULL, // default security attribute
+                                     TRUE, // manual-reset event
+                                     FALSE, // initial state = non signaled
+                                     NULL); // unnamed event object
 
     if (mOverlapped.hEvent == NULL)
     {
@@ -49,6 +48,30 @@ OVERLAPPED* WinOverlapped::data()
     return &mOverlapped;
 }
 
+std::pair<std::error_code, std::string> WinOverlapped::waitForCompletion(DWORD waitMs)
+{
+    const auto waitResult = WaitForSingleObject(mOverlapped.hEvent, waitMs);
+    switch (waitResult)
+    {
+        case WAIT_OBJECT_0:
+            return {std::error_code{}, ""};
+        case WAIT_TIMEOUT:
+            return {std::make_error_code(std::errc::timed_out),
+                    "wait timeout: " + std::to_string(waitMs)};
+        case WAIT_ABANDONED:
+            return {std::make_error_code(std::errc::not_connected), "wait abandoned"};
+        case WAIT_FAILED:
+        {
+            const auto lastError = GetLastError();
+            return {std::make_error_code(std::errc::not_connected),
+                    "wait failed error " + std::to_string(lastError) + " " +
+                        mega::winErrorMessage(lastError)};
+        }
+        default:
+            return {std::make_error_code(std::errc::not_connected), "wait error"};
+    }
+}
+
 NamedPipe::NamedPipe(NamedPipe&& other)
 {
     this->mPipeHandle = other.mPipeHandle;
@@ -61,7 +84,6 @@ NamedPipe::~NamedPipe()
     if (mPipeHandle != INVALID_HANDLE_VALUE)
     {
         CloseHandle(mPipeHandle);
-        LOG_verbose << "endpoint " << mName << "_" << mPipeHandle << " closed";
     }
 }
 
@@ -108,7 +130,7 @@ bool NamedPipe::doOverlappedOperation(std::function<bool(OVERLAPPED*)> op,
         return false;
     }
 
-    WinOverlapped overlapped;
+    WinOverlapped overlapped{};
     if (!overlapped.isValid())
     {
         return false;
@@ -117,34 +139,39 @@ bool NamedPipe::doOverlappedOperation(std::function<bool(OVERLAPPED*)> op,
     // Call Op.
     if (op(overlapped.data()))
     {
-        return true;
+        return true; // Completed
     }
-
-    // Error
-    auto lastError = GetLastError();
-    if (lastError!= ERROR_IO_PENDING)
+    else if (const auto lastError = GetLastError();
+             lastError != ERROR_IO_PENDING) // Fail with other errors
     {
-        LOG_err << mName << ": " << opStr << " pipe failed. error=" << lastError << " " << mega::winErrorMessage(lastError);
+        LOG_err << mName << ": " << opStr << " pipe failed. error=" << lastError << " "
+                << mega::winErrorMessage(lastError);
         return false;
     }
 
-    // Wait op to complete. Negative timeout is infinite
+    // Wait
     DWORD waitTimeout = timeout.count() < 0 ? INFINITE : static_cast<DWORD>(timeout.count());
+    if (const auto& [error, errorText] = overlapped.waitForCompletion(waitTimeout); error)
+    {
+        LOG_verbose << mName << ": " << opStr << " " << errorText;
+        return false;
+    }
+
+    // Get result
     DWORD numberOfBytesTransferred = 0;
-    bool success = GetOverlappedResultEx(mPipeHandle,
-                                         overlapped.data(),
-                                         &numberOfBytesTransferred,
-                                         waitTimeout,
-                                         false);
+    bool success = GetOverlappedResult(mPipeHandle,
+                                       overlapped.data(),
+                                       &numberOfBytesTransferred,
+                                       false /*bWait*/);
 
     if (!success)
     {
-        lastError = GetLastError();
+        const auto lastError = GetLastError();
         LOG_err << mName << ": " << opStr << " pipe fail to complete error=" << lastError << " " << mega::winErrorMessage(lastError);
         return false;
     }
 
-    // IO completed
+    // Completed
     return true;
 }
 

src/win32/gfx/worker/comms_client.cpp

@@ -61,8 +61,6 @@ std::pair<CommError, HANDLE> GfxCommunicationsClient::doConnect(LPCTSTR pipeName
         }
     }
 
-    LOG_verbose << "Connected Handle:" << hPipe << " error: " << static_cast<int>(error);
-
     return {error, hPipe};
 }
 

tools/gfxworker/src/win32/server.cpp

@@ -13,9 +13,7 @@ ServerNamedPipe::~ServerNamedPipe()
 {
     if (isValid())
     {
-        LOG_verbose << mName << "Endpoint server flush";
         FlushFileBuffers(mPipeHandle);
-        LOG_verbose << mName << "Endpoint server disconnect";
         DisconnectNamedPipe(mPipeHandle);
     }
 }
@@ -25,58 +23,53 @@ void ServerWin32::operator()()
     serverListeningLoop();
 }
 
-std::error_code ServerWin32::waitForClient(HANDLE hPipe, OVERLAPPED* overlapped)
+std::error_code ServerWin32::waitForClient(HANDLE hPipe, WinOverlapped& overlapped)
 {
     assert(hPipe != INVALID_HANDLE_VALUE);
-    assert(overlapped);
+    assert(overlapped.isValid());
 
     // Wait for the client to connect asynchronous; if it succeeds,
     // the function returns a nonzero value.
     // If the function returns zero,
-    //         GetLastError returns ERROR_IO_PENDING, the IO is connected
-    //         GetLastError returns ERROR_PIPE_CONNECTED, the IO is pending
-    bool success = ConnectNamedPipe(hPipe, overlapped);
+    //         GetLastError returns ERROR_PIPE_CONNECTED, the IO is connected
+    //         GetLastError returns ERROR_IO_PENDING, the IO is pending
+    bool success = ConnectNamedPipe(hPipe, overlapped.data());
     if (success)
     {
         LOG_verbose << "Client connected";
         return OK;
     }
 
-    if (!success && GetLastError() == ERROR_PIPE_CONNECTED)
+    if (GetLastError() == ERROR_PIPE_CONNECTED)
     {
         LOG_verbose << "Client connected";
         return OK;
     }
 
-    if (!success && GetLastError() != ERROR_IO_PENDING)
+    if (GetLastError() != ERROR_IO_PENDING)
     {
         LOG_verbose << "Client couldn't connect, error=" << GetLastError() << " " << mega::winErrorMessage(GetLastError());
         return std::make_error_code(std::errc::not_connected);
     }
 
-    // IO_PENDING
+    // Wait
+    if (auto [error, errorText] = overlapped.waitForCompletion(mWaitMs); error)
+    {
+        LOG_verbose << "Client " << errorText;
+        return error;
+    }
+
+    // Get result
     DWORD numberOfBytesTransferred = 0;
-    if (GetOverlappedResultEx(
-        hPipe,
-        overlapped,
-        &numberOfBytesTransferred,
-        mWaitMs,
-        false))
+    if (GetOverlappedResult(hPipe, overlapped.data(), &numberOfBytesTransferred, false /*bWait*/))
     {
         LOG_verbose << "Client connected";
         return OK;
     }
 
-    if (GetLastError() == WAIT_TIMEOUT)
-    {
-        LOG_verbose << "Wait client connecting Timeout";
-        return std::make_error_code(std::errc::timed_out);
-    }
-    else
-    {
-        LOG_verbose << "Client couldn't connect, error=" << GetLastError() << " " << mega::winErrorMessage(GetLastError());
-        return std::make_error_code(std::errc::not_connected);
-    }
+    LOG_verbose << "Client couldn't connect, error=" << GetLastError() << " "
+                << mega::winErrorMessage(GetLastError());
+    return std::make_error_code(std::errc::not_connected);
 }
 
 void ServerWin32::serverListeningLoop()
@@ -87,15 +80,15 @@ void ServerWin32::serverListeningLoop()
         return;
     }
 
+    LOG_verbose << "server awaiting client connection";
+
     const auto fullPipeName = win_utils::toFullPipeName(mPipeName);
 
     // first instance to prevent two processes create the same pipe
     DWORD firstInstance = FILE_FLAG_FIRST_PIPE_INSTANCE;
     const DWORD BUFSIZE = 512;
     for (;;)
     {
-        LOG_verbose << "server awaiting client connection";
-
         auto hPipe = CreateNamedPipe(
             fullPipeName.c_str(),     // pipe name
             PIPE_ACCESS_DUPLEX |      // read/write access
@@ -120,7 +113,7 @@ void ServerWin32::serverListeningLoop()
         firstInstance = 0;
 
         bool stopRunning = false;
-        auto err_code = waitForClient(hPipe, overlapped.data());
+        auto err_code = waitForClient(hPipe, overlapped);
         if (err_code)
         {
             // if has timeout and expires, we'll stop running

tools/gfxworker/src/win32/server.h

@@ -49,7 +49,7 @@ class ServerWin32
 
     void serverListeningLoop();
 
-    std::error_code waitForClient(HANDLE hPipe, OVERLAPPED* overlapped);
+    std::error_code waitForClient(HANDLE hPipe, WinOverlapped& overlapped);
 
     std::unique_ptr<RequestProcessor> mRequestProcessor;