Merge pull request #1266 from eclipse-iceoryx/iox-#1264-threadsafe-ha…

…ndle-translator-for-windows

Iox #1264 threadsafe handle translator for windows

iceoryx_hoofs/platform/win/include/iceoryx_hoofs/platform/handle_translator.hpp

@@ -1,4 +1,4 @@
-// Copyright (c) 2021 by Apex.AI Inc. All rights reserved.
+// Copyright (c) 2021 - 2022 by Apex.AI Inc. All rights reserved.
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -17,7 +17,9 @@
 #define IOX_HOOFS_WIN_PLATFORM_HANDLE_TRANSLATOR_HPP
 
 #include "iceoryx_hoofs/platform/windows.hpp"
-#include <vector>
+#include <map>
+#include <mutex>
+#include <queue>
 
 /// @brief In windows file handles have the type HANDLE (void*) in linux it is
 ///        usually an int. To establish the portability we keep track of the
@@ -28,14 +30,25 @@
 class HandleTranslator
 {
   public:
+    HandleTranslator(const HandleTranslator&) = delete;
+    HandleTranslator(HandleTranslator&&) = delete;
+    HandleTranslator& operator=(const HandleTranslator&) = delete;
+    HandleTranslator& operator=(HandleTranslator&&) = delete;
+    ~HandleTranslator() = default;
+
     static HandleTranslator& getInstance() noexcept;
-    HANDLE get(const int handle) const noexcept;
-    int add(HANDLE handle) noexcept;
-    void remove(const int handle) noexcept;
+    HANDLE get(const int linuxFd) const noexcept;
+    int add(HANDLE windowsHandle) noexcept;
+    void remove(const int linuxFd) noexcept;
 
   private:
     HandleTranslator() noexcept = default;
-    std::vector<HANDLE> m_handleList;
+
+    static constexpr int INVALID_LINUX_FD = -1;
+    int m_currentLinuxFileHandle = 0;
+    mutable std::mutex m_mtx;
+    std::map<int, HANDLE> m_linuxToWindows;
+    std::queue<int> m_freeFileDescriptors;
 };
 
 #endif

iceoryx_hoofs/platform/win/source/handle_translator.cpp

@@ -1,4 +1,4 @@
-// Copyright (c) 2021 by Apex.AI Inc. All rights reserved.
+// Copyright (c) 2021 - 2022 by Apex.AI Inc. All rights reserved.
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -16,33 +16,78 @@
 
 #include "iceoryx_hoofs/platform/handle_translator.hpp"
 
+#include <iostream>
+
+constexpr int HandleTranslator::INVALID_LINUX_FD;
+
 HandleTranslator& HandleTranslator::getInstance() noexcept
 {
     static HandleTranslator globalHandleTranslator;
     return globalHandleTranslator;
 }
 
-HANDLE HandleTranslator::get(const int handle) const noexcept
+HANDLE HandleTranslator::get(const int linuxFd) const noexcept
 {
-    return m_handleList[static_cast<size_t>(handle)];
+    if (linuxFd == INVALID_LINUX_FD)
+    {
+        return INVALID_HANDLE_VALUE;
+    }
+
+    std::lock_guard<std::mutex> lock(m_mtx);
+
+    auto iter = m_linuxToWindows.find(linuxFd);
+    if (iter == m_linuxToWindows.end())
+    {
+        std::cerr << "Cannot acquire windows file handle for not registered linux file descriptor " << linuxFd
+                  << std::endl;
+        return INVALID_HANDLE_VALUE;
+    }
+
+    return iter->second;
 }
 
-int HandleTranslator::add(HANDLE handle) noexcept
+int HandleTranslator::add(HANDLE windowsHandle) noexcept
 {
-    for (int64_t limit = m_handleList.size(), k = 0; k < limit; ++k)
+    if (windowsHandle == INVALID_HANDLE_VALUE)
+    {
+        return INVALID_LINUX_FD;
+    }
+
+    std::lock_guard<std::mutex> lock(m_mtx);
+
+    int linuxFd = INVALID_LINUX_FD;
+    if (!m_freeFileDescriptors.empty())
+    {
+        linuxFd = m_freeFileDescriptors.front();
+        m_freeFileDescriptors.pop();
+    }
+    else
     {
-        if (m_handleList[k] == nullptr)
-        {
-            m_handleList[k] = handle;
-            return k;
-        }
+        linuxFd = m_currentLinuxFileHandle;
+        ++m_currentLinuxFileHandle;
     }
 
-    m_handleList.emplace_back(handle);
-    return m_handleList.size() - 1;
+    m_linuxToWindows[linuxFd] = windowsHandle;
+    return linuxFd;
 }
 
-void HandleTranslator::remove(const int handle) noexcept
+void HandleTranslator::remove(const int linuxFd) noexcept
 {
-    m_handleList[static_cast<uint64_t>(handle)] = nullptr;
+    if (linuxFd == INVALID_LINUX_FD)
+    {
+        return;
+    }
+
+    std::lock_guard<std::mutex> lock(m_mtx);
+
+    auto iter = m_linuxToWindows.find(linuxFd);
+    if (iter == m_linuxToWindows.end())
+    {
+        std::cerr << "Unable to release not registered file handle " << linuxFd << " since it was not acquired"
+                  << std::endl;
+        return;
+    }
+
+    m_linuxToWindows.erase(iter);
+    m_freeFileDescriptors.emplace(linuxFd);
 }

iceoryx_posh/test/moduletests/test_posh_runtime.cpp

@@ -143,8 +143,7 @@ TEST_F(PoshRuntime_test, NoAppName)
     ::testing::Test::RecordProperty("TEST_ID", "e053d114-c79c-4391-91e1-8fcfe90ee8e4");
     const iox::RuntimeName_t invalidAppName("");
 
-    EXPECT_DEATH({ PoshRuntime::initRuntime(invalidAppName); },
-                 "Cannot initialize runtime. Application name must not be empty!");
+    EXPECT_DEATH({ PoshRuntime::initRuntime(invalidAppName); }, "");
 }
 
 // To be able to test the singleton and avoid return the exisiting instance, we don't use the test fixture

tools/ci/build-test-windows.ps1

@@ -30,7 +30,7 @@ if ($?) { Write-Host "running tests (excluding timing_tests)" }
 if ($?) { build\hoofs\test\Debug\hoofs_moduletests.exe --gtest_filter="-SharedMemory_Test.CreateOrOpenCreatesShmWhenShmDoesNotExist:SharedMemory_Test.OpenFailsWhenShmDoesNotExist:*TimingTest*" }
 if ($?) { build\hoofs\test\Debug\hoofs_integrationtests.exe }
 if ($?) { build\binding_c\test\Debug\binding_c_moduletests.exe --gtest_filter="-BindingC_Runtime_test.RuntimeNameLengthIsOutOfLimit:BindingC_Runtime_test.RuntimeNameIsNullptr:*TimingTest*" }
-if ($?) { build\posh\test\Debug\posh_moduletests.exe --gtest_filter="-ChunkHeader_test.ChunkHeaderBinaryCompatibilityCheck:TomlGatewayConfigParserSuiteTest*:IceoryxRoudiApp_test*:PoshRuntime_test.NoAppName:ValidTest*:ParseAllMalformedInput*:*TimingTest*" }
+if ($?) { build\posh\test\Debug\posh_moduletests.exe --gtest_filter="-ChunkHeader_test.ChunkHeaderBinaryCompatibilityCheck:TomlGatewayConfigParserSuiteTest*:IceoryxRoudiApp_test.ConstructorCalledWithArgUniqueIdTwoTimesReturnError:IceoryxRoudiApp_test.ConstructorCalledWithArgVersionSetRunVariableToFalse:ValidTest*:ParseAllMalformedInput*:*TimingTest*" }
 if ($?) { build\posh\test\Debug\posh_integrationtests.exe --gtest_filter="-ChunkBuildingBlocks_IntegrationTest.TwoHopsThreeThreadsNoSoFi:*TimingTest*" }
 
 exit $LASTEXITCODE