From c9c4c9ba8c00e19e937d04d721e8d40e5f549907 Mon Sep 17 00:00:00 2001 From: Andrey Nekrasov Date: Fri, 18 Dec 2020 15:25:58 +0300 Subject: [PATCH] ImageResizer: fix crash on using UseNewSettings (#8661) * ImageResizer: fix crash on using UseNewSettings - add unit test to InteropTests --- .github/actions/spell-check/expect.txt | 1 + src/common/interop/PowerToysInterop.vcxproj | 8 +- .../interop/PowerToysInterop.vcxproj.filters | 6 - .../interop/interop-tests/InteropTests.cs | 3 + src/common/interop/interop.cpp | 140 +++++++++++++++++- src/common/interop/interop.h | 135 ----------------- src/common/interop/keyboard_layout.cpp | 4 + src/common/interop/keyboard_layout_impl.h | 3 - src/common/utils/os-detect.h | 2 +- 9 files changed, 150 insertions(+), 152 deletions(-) delete mode 100644 src/common/interop/interop.h diff --git a/.github/actions/spell-check/expect.txt b/.github/actions/spell-check/expect.txt index 16733fffc4..b895649284 100644 --- a/.github/actions/spell-check/expect.txt +++ b/.github/actions/spell-check/expect.txt @@ -342,6 +342,7 @@ CPPARM cppblog cppm cpprestsdk +cppruntime cppstd cppwinrt CPPx diff --git a/src/common/interop/PowerToysInterop.vcxproj b/src/common/interop/PowerToysInterop.vcxproj index e0958df04c..c5c59fe29c 100644 --- a/src/common/interop/PowerToysInterop.vcxproj +++ b/src/common/interop/PowerToysInterop.vcxproj @@ -84,9 +84,7 @@ - - @@ -96,12 +94,10 @@ - false - false + false - false - false + false diff --git a/src/common/interop/PowerToysInterop.vcxproj.filters b/src/common/interop/PowerToysInterop.vcxproj.filters index fe86d91de6..c68a2dff76 100644 --- a/src/common/interop/PowerToysInterop.vcxproj.filters +++ b/src/common/interop/PowerToysInterop.vcxproj.filters @@ -15,9 +15,6 @@ - - Header Files - Header Files @@ -30,9 +27,6 @@ Header Files - - Header Files - diff --git a/src/common/interop/interop-tests/InteropTests.cs b/src/common/interop/interop-tests/InteropTests.cs index aa01689c42..3e71c09c58 100644 --- a/src/common/interop/interop-tests/InteropTests.cs +++ b/src/common/interop/interop-tests/InteropTests.cs @@ -22,6 +22,9 @@ namespace Microsoft.Interop.Tests [TestInitialize] public void Initialize() { + // Make sure we don't crash + Assert.IsTrue(CommonManaged.ShouldNewSettingsBeUsed() == CommonManaged.ShouldNewSettingsBeUsed()); + ClientPipe = new TwoWayPipeMessageIPCManaged(ClientSidePipe, ServerSidePipe, null); } diff --git a/src/common/interop/interop.cpp b/src/common/interop/interop.cpp index 16c4ee80d8..ab0eb2ebb1 100644 --- a/src/common/interop/interop.cpp +++ b/src/common/interop/interop.cpp @@ -1,4 +1,142 @@ #include "pch.h" -#include "interop.h" +#include +#include +#include +#include "keyboard_layout.h" +#include "two_way_pipe_message_ipc.h" +#include "shared_constants.h" +// We cannot use C++/WinRT APIs when compiled with /clr (we'll get a runtime crash). os-detect API is used +// in both native C++ and C++/CX. +// We also cannot compile it as a library, since we use different cppruntime linkage in C++/CX and native C++. +// Therefore the simplest way is to compile these functions as native using the pragmas below. +#pragma managed(push, off) +#include "../utils/os-detect.h" +#pragma managed(pop) + +#include + +using namespace System; +using namespace System::Runtime::InteropServices; + +// https://docs.microsoft.com/en-us/cpp/dotnet/how-to-wrap-native-class-for-use-by-csharp?view=vs-2019 +namespace interop +{ +public + ref class LayoutMapManaged + { + public: + LayoutMapManaged() : + _map(new LayoutMap) {} + + ~LayoutMapManaged() + { + delete _map; + } + + String ^ GetKeyName(DWORD key) { + return gcnew String(_map->GetKeyName(key).c_str()); + } + + void Updatelayout() + { + _map->UpdateLayout(); + } + + protected: + !LayoutMapManaged() + { + delete _map; + } + + private: + LayoutMap* _map; + }; + +public + ref class TwoWayPipeMessageIPCManaged + { + public: + delegate void ReadCallback(String ^ message); + + TwoWayPipeMessageIPCManaged(String ^ inputPipeName, String ^ outputPipeName, ReadCallback ^ callback) + { + _wrapperCallback = gcnew InternalReadCallback(this, &TwoWayPipeMessageIPCManaged::ReadCallbackHelper); + _callback = callback; + + TwoWayPipeMessageIPC::callback_function cb = nullptr; + if (callback != nullptr) + { + cb = (TwoWayPipeMessageIPC::callback_function)(void*)Marshal::GetFunctionPointerForDelegate(_wrapperCallback); + } + _pipe = new TwoWayPipeMessageIPC( + msclr::interop::marshal_as(inputPipeName), + msclr::interop::marshal_as(outputPipeName), + cb); + } + + ~TwoWayPipeMessageIPCManaged() + { + delete _pipe; + } + + void Send(String ^ msg) + { + _pipe->send(msclr::interop::marshal_as(msg)); + } + + void Start() + { + _pipe->start(nullptr); + } + + void End() + { + _pipe->end(); + } + + protected: + !TwoWayPipeMessageIPCManaged() + { + delete _pipe; + } + + private: + delegate void InternalReadCallback(const std::wstring& msg); + + TwoWayPipeMessageIPC* _pipe; + ReadCallback ^ _callback; + InternalReadCallback ^ _wrapperCallback; + + void ReadCallbackHelper(const std::wstring& msg) + { + _callback(gcnew String(msg.c_str())); + } + }; + +public + ref class CommonManaged + { + public: + static String ^ GetProductVersion() { + return gcnew String(get_product_version().c_str()); + } + + static bool ShouldNewSettingsBeUsed() + { + return UseNewSettings(); + } + }; + +public + ref class Constants + { + public: + literal int VK_WIN_BOTH = CommonSharedConstants::VK_WIN_BOTH; + + static String ^ PowerLauncherSharedEvent() { + return gcnew String(CommonSharedConstants::POWER_LAUNCHER_SHARED_EVENT); + } + }; +} diff --git a/src/common/interop/interop.h b/src/common/interop/interop.h deleted file mode 100644 index 183de8051b..0000000000 --- a/src/common/interop/interop.h +++ /dev/null @@ -1,135 +0,0 @@ -#pragma once - -#include -#include -#include -#include "keyboard_layout.h" -#include "two_way_pipe_message_ipc.h" -#include "shared_constants.h" -#include "../utils/os-detect.h" - -#include - -using namespace System; -using namespace System::Runtime::InteropServices; - -//https://docs.microsoft.com/en-us/cpp/dotnet/how-to-wrap-native-class-for-use-by-csharp?view=vs-2019 -namespace interop -{ -public - ref class LayoutMapManaged - { - public: - LayoutMapManaged() : - _map(new LayoutMap) {} - - ~LayoutMapManaged() - { - delete _map; - } - - String ^ GetKeyName(DWORD key) { - return gcnew String(_map->GetKeyName(key).c_str()); - } - - void Updatelayout() - { - _map->UpdateLayout(); - } - - protected: - !LayoutMapManaged() - { - delete _map; - } - - private: - LayoutMap* _map; - }; - -public - ref class TwoWayPipeMessageIPCManaged - { - public: - delegate void ReadCallback(String ^ message); - - TwoWayPipeMessageIPCManaged(String ^ inputPipeName, String ^ outputPipeName, ReadCallback ^ callback) - { - _wrapperCallback = gcnew InternalReadCallback(this, &TwoWayPipeMessageIPCManaged::ReadCallbackHelper); - _callback = callback; - - TwoWayPipeMessageIPC::callback_function cb = nullptr; - if (callback != nullptr) - { - cb = (TwoWayPipeMessageIPC::callback_function)(void*)Marshal::GetFunctionPointerForDelegate(_wrapperCallback); - } - _pipe = new TwoWayPipeMessageIPC( - msclr::interop::marshal_as(inputPipeName), - msclr::interop::marshal_as(outputPipeName), - cb); - } - - ~TwoWayPipeMessageIPCManaged() - { - delete _pipe; - } - - void Send(String ^ msg) - { - _pipe->send(msclr::interop::marshal_as(msg)); - } - - void Start() - { - _pipe->start(nullptr); - } - - void End() - { - _pipe->end(); - } - - protected: - !TwoWayPipeMessageIPCManaged() - { - delete _pipe; - } - - private: - delegate void InternalReadCallback(const std::wstring& msg); - - TwoWayPipeMessageIPC* _pipe; - ReadCallback ^ _callback; - InternalReadCallback ^ _wrapperCallback; - - void ReadCallbackHelper(const std::wstring& msg) - { - _callback(gcnew String(msg.c_str())); - } - }; - -public - ref class CommonManaged - { - public: - static String ^ GetProductVersion() { - return gcnew String(get_product_version().c_str()); - } - - static bool ShouldNewSettingsBeUsed() - { - return UseNewSettings(); - } - }; - -public - ref class Constants - { - public: - literal int VK_WIN_BOTH = CommonSharedConstants::VK_WIN_BOTH; - - static String ^ PowerLauncherSharedEvent() { - return gcnew String(CommonSharedConstants::POWER_LAUNCHER_SHARED_EVENT); - } - }; -} diff --git a/src/common/interop/keyboard_layout.cpp b/src/common/interop/keyboard_layout.cpp index ac0ff19c4b..8f00009f40 100644 --- a/src/common/interop/keyboard_layout.cpp +++ b/src/common/interop/keyboard_layout.cpp @@ -2,6 +2,10 @@ #include "keyboard_layout_impl.h" #include "shared_constants.h" +#include + +using namespace winrt; + LayoutMap::LayoutMap() : impl(new LayoutMap::LayoutMapImpl()) { diff --git a/src/common/interop/keyboard_layout_impl.h b/src/common/interop/keyboard_layout_impl.h index dc1802e537..5de98f8c66 100644 --- a/src/common/interop/keyboard_layout_impl.h +++ b/src/common/interop/keyboard_layout_impl.h @@ -3,9 +3,6 @@ #include #include #include -#include - -using namespace winrt; // Wrapper class to handle keyboard layout class LayoutMap::LayoutMapImpl diff --git a/src/common/utils/os-detect.h b/src/common/utils/os-detect.h index a3f7eda288..6fad87a9b3 100644 --- a/src/common/utils/os-detect.h +++ b/src/common/utils/os-detect.h @@ -28,4 +28,4 @@ inline bool Is19H1OrHigher() inline bool UseNewSettings() { return Is19H1OrHigher(); -} \ No newline at end of file +}