From 1e53725456dee04fd8fa80aef5ba9a5115566e7c Mon Sep 17 00:00:00 2001 From: Zihao Date: Fri, 23 Oct 2026 19:13:44 +0800 Subject: [PATCH] fix(connectivity_plus): defer Windows COM cleanup to final manager owner Stop the listener when the plugin is destroyed, but leave COM cleanup to the final shared NetworkManager owner. The event-stream handler can outlive the plugin, and both destructors previously called Cleanup for one Init. Add a standalone native regression using the real plugin and COM apartment. On Windows 10 build 19045, the original source fails with listener_stopped false and CO_E_NOTINITIALIZED (0x800401f0); the patched source passes with listener_stopped true and S_OK. Both Release executables build successfully. Validation: Windows native before/after regression, 16 package Flutter tests, flutter analyze --no-pub, focused clang-format and git diff --check passed. --- .../windows/connectivity_plus_plugin.cpp | 4 +- .../windows/tests/CMakeLists.txt | 37 +++++++++++++++++ .../connectivity_plus/windows/tests/README.md | 31 ++++++++++++++ .../tests/connectivity_cleanup_test.cpp | 41 +++++++++++++++++++ 4 files changed, 112 insertions(+), 1 deletion(-) create mode 100644 packages/connectivity_plus/connectivity_plus/windows/tests/CMakeLists.txt create mode 100644 packages/connectivity_plus/connectivity_plus/windows/tests/README.md create mode 100644 packages/connectivity_plus/connectivity_plus/windows/tests/connectivity_cleanup_test.cpp diff --git a/packages/connectivity_plus/connectivity_plus/windows/connectivity_plus_plugin.cpp b/packages/connectivity_plus/connectivity_plus/windows/connectivity_plus_plugin.cpp index 76b7a26b72..847a120349 100644 --- a/packages/connectivity_plus/connectivity_plus/windows/connectivity_plus_plugin.cpp +++ b/packages/connectivity_plus/connectivity_plus/windows/connectivity_plus_plugin.cpp @@ -82,7 +82,9 @@ ConnectivityPlusWindowsPlugin::ConnectivityPlusWindowsPlugin() { } ConnectivityPlusWindowsPlugin::~ConnectivityPlusWindowsPlugin() { - manager->Cleanup(); + // The event stream can still own the manager. Stop callbacks now, and let + // the last shared owner release COM through NetworkManager's destructor. + manager->StopListen(); } std::shared_ptr diff --git a/packages/connectivity_plus/connectivity_plus/windows/tests/CMakeLists.txt b/packages/connectivity_plus/connectivity_plus/windows/tests/CMakeLists.txt new file mode 100644 index 0000000000..bc5adbe131 --- /dev/null +++ b/packages/connectivity_plus/connectivity_plus/windows/tests/CMakeLists.txt @@ -0,0 +1,37 @@ +cmake_minimum_required(VERSION 3.15) +project(connectivity_cleanup_test LANGUAGES CXX) +enable_testing() + +if(NOT WIN32) + message(FATAL_ERROR "This regression exercises real Windows COM.") +endif() +foreach(required FLUTTER_WRAPPER_DIR FLUTTER_ENGINE_DIR) + if(NOT IS_DIRECTORY "${${required}}") + message(FATAL_ERROR "Set ${required} to the existing Flutter SDK directory.") + endif() +endforeach() + +add_library(flutter_wrapper_plugin STATIC + "${FLUTTER_WRAPPER_DIR}/core_implementations.cc" + "${FLUTTER_WRAPPER_DIR}/plugin_registrar.cc" + "${FLUTTER_WRAPPER_DIR}/standard_codec.cc") +target_compile_features(flutter_wrapper_plugin PUBLIC cxx_std_17) +target_compile_options(flutter_wrapper_plugin PRIVATE /utf-8) +target_include_directories(flutter_wrapper_plugin PUBLIC + "${FLUTTER_WRAPPER_DIR}/include" "${FLUTTER_ENGINE_DIR}") +target_link_libraries(flutter_wrapper_plugin PUBLIC + "${FLUTTER_ENGINE_DIR}/flutter_windows.dll.lib") + +add_executable(connectivity_cleanup_test + connectivity_cleanup_test.cpp "../network_manager.cpp") +target_compile_features(connectivity_cleanup_test PRIVATE cxx_std_17) +target_compile_options(connectivity_cleanup_test PRIVATE /utf-8) +target_compile_definitions(connectivity_cleanup_test PRIVATE FLUTTER_PLUGIN_IMPL) +target_link_libraries(connectivity_cleanup_test PRIVATE + flutter_wrapper_plugin ole32 oleaut32 uuid iphlpapi) +add_custom_command(TARGET connectivity_cleanup_test POST_BUILD + COMMAND ${CMAKE_COMMAND} -E copy_if_different + "${FLUTTER_ENGINE_DIR}/flutter_windows.dll" + "$") + +add_test(NAME preserves_callers_com_apartment COMMAND connectivity_cleanup_test) diff --git a/packages/connectivity_plus/connectivity_plus/windows/tests/README.md b/packages/connectivity_plus/connectivity_plus/windows/tests/README.md new file mode 100644 index 0000000000..ce7075cc35 --- /dev/null +++ b/packages/connectivity_plus/connectivity_plus/windows/tests/README.md @@ -0,0 +1,31 @@ +# Windows COM cleanup regression + +This standalone CTest compiles the actual plugin and `NetworkManager` with the +Flutter Windows wrapper. It exercises the real COM apartment and network +listener without launching a Flutter app. It does not change production builds +or require an additional test framework. + +Use an existing Flutter Windows project's `windows/flutter/ephemeral` directory, +generated by a Windows build, for the headers, wrapper sources, and engine DLL. +From the `connectivity_plus` package directory on Windows: + +```powershell +$engine = "/windows/flutter/ephemeral" +cmake -S windows/tests -B build/windows-cleanup-test -A x64 ` + "-DFLUTTER_ENGINE_DIR=$engine" ` + "-DFLUTTER_WRAPPER_DIR=$engine/cpp_client_wrapper" +cmake --build build/windows-cleanup-test --config Release +ctest --test-dir build/windows-cleanup-test -C Release --output-on-failure +``` + +The caller first initializes its own COM apartment. The plugin and the event +stream then share a manager. After the plugin is destroyed, the listener must +already be stopped but the manager must still be usable. Releasing the stream's +owner must preserve the caller's apartment (`S_OK`). + +With the original plugin destructor calling `Cleanup()`, this test fails with +`listener_stopped=false caller_apartment_hresult=0x800401f0` +(`CO_E_NOTINITIALIZED`): the plugin and the manager each call `CoUninitialize()` +for a single manager initialization. With `StopListen()` in the plugin +destructor, it passes with `listener_stopped=true caller_apartment_hresult=0x0`. +A COM initialization or listener setup failure is a test failure, not a pass. diff --git a/packages/connectivity_plus/connectivity_plus/windows/tests/connectivity_cleanup_test.cpp b/packages/connectivity_plus/connectivity_plus/windows/tests/connectivity_cleanup_test.cpp new file mode 100644 index 0000000000..885134fd9c --- /dev/null +++ b/packages/connectivity_plus/connectivity_plus/windows/tests/connectivity_cleanup_test.cpp @@ -0,0 +1,41 @@ +// Include the actual plugin class, which is private to this translation unit. +#include "../connectivity_plus_plugin.cpp" + +#include + +int main() { + if (FAILED(CoInitializeEx(nullptr, COINIT_APARTMENTTHREADED))) { + std::cerr << "Could not initialize the caller's COM apartment." + << std::endl; + return 10; + } + + auto plugin = std::make_unique(); + // The event-channel handler retains this owner until channel teardown. + auto stream_owner = plugin->GetManager(); + if (!stream_owner->StartListen([] {})) { + std::cerr << "Could not start the initial listener." << std::endl; + return 11; + } + + plugin.reset(); + // Teardown must unregister the old listener without invalidating the shared + // manager. Starting another listener checks both conditions. + const bool listener_stopped = stream_owner->StartListen([] {}); + if (listener_stopped) { + stream_owner->StopListen(); + } + stream_owner.reset(); + + APTTYPE type; + APTTYPEQUALIFIER qualifier; + const HRESULT apartment = CoGetApartmentType(&type, &qualifier); + std::cout << "listener_stopped=" << std::boolalpha << listener_stopped + << " caller_apartment_hresult=0x" << std::hex + << static_cast(apartment) << std::endl; + if (SUCCEEDED(apartment)) { + CoUninitialize(); + } + + return listener_stopped && apartment == S_OK ? 0 : 1; +}