asan: changing how stub input platform is allocated to avoid statics leaking into libraries that include it

Author: mattkae

tests/include/mir_test_framework/stub_input_platform.h

@@ -36,11 +36,22 @@ class InputDevice;
 }
 namespace mir_test_framework
 {
+
+class DeviceStore
+{
+public:
+    virtual ~DeviceStore() = default;
+    virtual void foreach_device(std::function<void(std::weak_ptr<mir::input::InputDevice> const&)> const&) = 0;
+    virtual void clear() = 0;
+};
+
 class FakeInputDevice;
 class StubInputPlatform : public mir::input::Platform
 {
 public:
-    explicit StubInputPlatform(std::shared_ptr<mir::input::InputDeviceRegistry> const& input_device_registry);
+    explicit StubInputPlatform(
+        std::shared_ptr<mir::input::InputDeviceRegistry> const& input_device_registry,
+        std::shared_ptr<DeviceStore> const& device_store);
     ~StubInputPlatform();
 
     std::shared_ptr<mir::dispatch::Dispatchable> dispatchable() override;
@@ -49,18 +60,16 @@ class StubInputPlatform : public mir::input::Platform
     void pause_for_config() override;
     void continue_after_config() override;
 
-    static void add(std::shared_ptr<mir::input::InputDevice> const& dev);
-    static void remove(std::shared_ptr<mir::input::InputDevice> const& dev);
-    static void register_dispatchable(std::shared_ptr<mir::dispatch::Dispatchable> const& queue);
-    static void unregister_dispatchable(std::shared_ptr<mir::dispatch::Dispatchable> const& queue);
+    void add(std::shared_ptr<mir::input::InputDevice> const& dev);
+    void remove(std::shared_ptr<mir::input::InputDevice> const& dev);
+    void register_dispatchable(std::shared_ptr<mir::dispatch::Dispatchable> const& queue);
+    void unregister_dispatchable(std::shared_ptr<mir::dispatch::Dispatchable> const& queue);
 
 private:
     std::shared_ptr<mir::dispatch::MultiplexingDispatchable> const platform_dispatchable;
     std::shared_ptr<mir::dispatch::ActionQueue> const platform_queue;
     std::shared_ptr<mir::input::InputDeviceRegistry> const registry;
-    static std::atomic<StubInputPlatform*> stub_input_platform;
-    static std::vector<std::weak_ptr<mir::input::InputDevice>> device_store;
-    static std::mutex device_store_guard;
+    std::shared_ptr<DeviceStore> const device_store;
 };
 
 }

tests/include/mir_test_framework/stub_input_platform_accessor.h

@@ -0,0 +1,40 @@
+/*
+ * Copyright © Canonical Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 or 3,
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef MIR_TEST_FRAMEWORK_STUB_INPUT_PLATFORM_ACCESSOR_H
+#define MIR_TEST_FRAMEWORK_STUB_INPUT_PLATFORM_ACCESSOR_H
+
+#include "stub_input_platform.h"
+
+namespace mir_test_framework
+{
+class StubInputPlatformAccessor
+{
+public:
+    static mir::UniqueModulePtr<mir::input::Platform> get(std::shared_ptr<mir::input::InputDeviceRegistry> const& input_device_registry);
+    static void add(std::shared_ptr<mir::input::InputDevice> const& dev);
+    static void remove(std::shared_ptr<mir::input::InputDevice> const& dev);
+    static void register_dispatchable(std::shared_ptr<mir::dispatch::Dispatchable> const& queue);
+    static void unregister_dispatchable(std::shared_ptr<mir::dispatch::Dispatchable> const& queue);
+    static void clear();
+
+private:
+    static std::atomic<StubInputPlatform*> stub_input_platform;
+
+};
+}
+
+#endif //MIR_TEST_FRAMEWORK_STUB_INPUT_PLATFORM_ACCESSOR_H

tests/mir_test_framework/CMakeLists.txt

@@ -21,6 +21,17 @@ add_compile_definitions(
 string (REPLACE " -flto " " " CMAKE_C_FLAGS ${CMAKE_C_FLAGS})
 string (REPLACE " -flto " " " CMAKE_CXX_FLAGS ${CMAKE_CXX_FLAGS})
 
+add_library(mir-stub-input-platform OBJECT
+  stub_input_platform.cpp
+)
+
+target_link_libraries(mir-stub-input-platform
+  PUBLIC
+    mirplatform
+    mircommon
+    mircore
+)
+
 add_library(mir-public-test-framework OBJECT
   async_server_runner.cpp
   command_line_server_configuration.cpp
@@ -93,6 +104,7 @@ target_link_libraries(mir-umock-test-framework
 
 add_library(mir-test-framework-static STATIC
   $<TARGET_OBJECTS:mir-public-test-framework>
+  $<TARGET_OBJECTS:mir-stub-input-platform>
   $<TARGET_OBJECTS:mir-protected-test-framework>
   $<TARGET_OBJECTS:mir-public-test>
 )
@@ -115,8 +127,8 @@ add_library(
   mir-test-input-framework OBJECT
 
   stub_input.cpp
+  stub_input_platform_accessor.cpp
   fake_input_device_impl.cpp
-  stub_input_platform.cpp
   )
 
 target_link_libraries(mir-test-input-framework
@@ -129,7 +141,8 @@ target_link_libraries(mir-test-input-framework
 add_library(
   mirplatforminputstub MODULE
   $<TARGET_OBJECTS:mir-test-input-framework>
-  $<TARGET_OBJECTS:mirevdevutilsobjects>)
+  $<TARGET_OBJECTS:mirevdevutilsobjects>
+  $<TARGET_OBJECTS:mir-stub-input-platform>)
 target_link_libraries(mirplatforminputstub mircommon)
 
 set_target_properties(

tests/mir_test_framework/fake_input_device_impl.cpp

@@ -15,7 +15,7 @@
  */
 
 #include "fake_input_device_impl.h"
-#include "mir_test_framework/stub_input_platform.h"
+#include "mir_test_framework/stub_input_platform_accessor.h"
 
 #include "mir/input/input_device.h"
 #include "mir/input/input_device_info.h"
@@ -45,12 +45,12 @@ namespace synthesis = mir::input::synthesis;
 mtf::FakeInputDeviceImpl::FakeInputDeviceImpl(mi::InputDeviceInfo const& info)
     : queue{std::make_shared<md::ActionQueue>()}, device{std::make_shared<InputDevice>(info, queue)}
 {
-    mtf::StubInputPlatform::add(device);
+    mtf::StubInputPlatformAccessor::add(device);
 }
 
 void mtf::FakeInputDeviceImpl::emit_device_removal()
 {
-    mtf::StubInputPlatform::remove(device);
+    mtf::StubInputPlatformAccessor::remove(device);
 }
 
 void mtf::FakeInputDeviceImpl::emit_runtime_error()
@@ -328,14 +328,14 @@ void mtf::FakeInputDeviceImpl::InputDevice::start(mi::InputSink* destination, mi
 {
     sink = destination;
     builder = event_builder;
-    mtf::StubInputPlatform::register_dispatchable(queue);
+    mtf::StubInputPlatformAccessor::register_dispatchable(queue);
 }
 
 void mtf::FakeInputDeviceImpl::InputDevice::stop()
 {
     sink = nullptr;
     builder = nullptr;
-    mtf::StubInputPlatform::unregister_dispatchable(queue);
+    mtf::StubInputPlatformAccessor::unregister_dispatchable(queue);
 }
 
 mi::OutputInfo mtf::FakeInputDeviceImpl::InputDevice::get_output_info() const

tests/mir_test_framework/stub_input.cpp

@@ -14,7 +14,7 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-#include "mir_test_framework/stub_input_platform.h"
+#include "mir_test_framework/stub_input_platform_accessor.h"
 #include "fake_input_device_impl.h"
 #include "mir/module_properties.h"
 #include "mir/assert_module_entry_point.h"
@@ -33,7 +33,7 @@ mir::UniqueModulePtr<mi::Platform> create_input_platform(
     std::shared_ptr<mi::InputReport> const& /*report*/)
 {
     mir::assert_entry_point_signature<mi::CreatePlatform>(&create_input_platform);
-    return mir::make_module_ptr<mtf::StubInputPlatform>(input_device_registry);
+    return mtf::StubInputPlatformAccessor::get(input_device_registry);
 }
 
 void add_input_platform_options(

tests/mir_test_framework/stub_input_platform.cpp

@@ -27,31 +27,29 @@ namespace mtf = mir_test_framework;
 namespace mi = mir::input;
 
 mtf::StubInputPlatform::StubInputPlatform(
-    std::shared_ptr<mi::InputDeviceRegistry> const& input_device_registry)
+    std::shared_ptr<mi::InputDeviceRegistry> const& input_device_registry,
+    std::shared_ptr<DeviceStore> const& device_store)
     : platform_dispatchable{std::make_shared<mir::dispatch::MultiplexingDispatchable>()},
     platform_queue{std::make_shared<mir::dispatch::ActionQueue>()},
-    registry(input_device_registry)
+    registry(input_device_registry),
+    device_store(device_store)
 {
-    stub_input_platform = this;
     platform_dispatchable->add_watch(platform_queue);
 }
 
 mtf::StubInputPlatform::~StubInputPlatform()
 {
-    std::lock_guard lk{device_store_guard};
-    device_store.clear();
-    stub_input_platform = nullptr;
+    device_store->clear();
 }
 
 void mtf::StubInputPlatform::start()
 {
-    std::lock_guard lk{device_store_guard};
-    for (auto const& dev : device_store)
+    device_store->foreach_device([&](auto const& dev)
     {
         auto device = dev.lock();
         if (device)
             registry->add_device(device);
-    }
+    });
 }
 
 std::shared_ptr<mir::dispatch::Dispatchable> mtf::StubInputPlatform::dispatchable()
@@ -61,13 +59,12 @@ std::shared_ptr<mir::dispatch::Dispatchable> mtf::StubInputPlatform::dispatchabl
 
 void mtf::StubInputPlatform::stop()
 {
-    std::lock_guard lk{device_store_guard};
-    for (auto const& dev : device_store)
+    device_store->foreach_device([&](auto const& dev)
     {
         auto device = dev.lock();
         if (device)
             registry->remove_device(device);
-    }
+    });
 }
 
 void mtf::StubInputPlatform::pause_for_config()
@@ -80,61 +77,28 @@ void mtf::StubInputPlatform::continue_after_config()
 
 void mtf::StubInputPlatform::add(std::shared_ptr<mir::input::InputDevice> const& dev)
 {
-    auto input_platform = stub_input_platform.load();
-    if (!input_platform)
-    {
-        std::lock_guard lk{device_store_guard};
-        device_store.push_back(dev);
-        return;
-    }
-
-    input_platform->platform_queue->enqueue(
-        [registry=input_platform->registry,dev]
+    platform_queue->enqueue(
+        [registry=registry,dev]
         {
             registry->add_device(dev);
         });
 }
 
 void mtf::StubInputPlatform::remove(std::shared_ptr<mir::input::InputDevice> const& dev)
 {
-    auto input_platform = stub_input_platform.load();
-    if (!input_platform)
-        BOOST_THROW_EXCEPTION(std::runtime_error("No stub input platform available"));
-
-    std::lock_guard lk{device_store_guard};
-    device_store.erase(
-        std::remove_if(begin(device_store),
-                       end(device_store),
-                       [dev](auto weak_dev)
-                       {
-                           return (weak_dev.lock() == dev);
-                       }),
-        end(device_store));
-    input_platform->platform_queue->enqueue(
-        [ registry = input_platform->registry, dev ]
+    platform_queue->enqueue(
+        [ registry = registry, dev ]
         {
             registry->remove_device(dev);
         });
 }
 
 void mtf::StubInputPlatform::register_dispatchable(std::shared_ptr<mir::dispatch::Dispatchable> const& queue)
 {
-    auto input_platform = stub_input_platform.load();
-    if (!input_platform)
-        BOOST_THROW_EXCEPTION(std::runtime_error("No stub input platform available"));
-
-    input_platform->platform_dispatchable->add_watch(queue);
+    platform_dispatchable->add_watch(queue);
 }
 
 void mtf::StubInputPlatform::unregister_dispatchable(std::shared_ptr<mir::dispatch::Dispatchable> const& queue)
 {
-    auto input_platform = stub_input_platform.load();
-    if (!input_platform)
-        BOOST_THROW_EXCEPTION(std::runtime_error("No stub input platform available"));
-
-    input_platform->platform_dispatchable->remove_watch(queue);
+    platform_dispatchable->remove_watch(queue);
 }
-
-std::atomic<mtf::StubInputPlatform*> mtf::StubInputPlatform::stub_input_platform{nullptr};
-std::vector<std::weak_ptr<mir::input::InputDevice>> mtf::StubInputPlatform::device_store;
-std::mutex mtf::StubInputPlatform::device_store_guard;