-
Notifications
You must be signed in to change notification settings - Fork 364
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Applications that do not explicitly call CoInitialize
may crash when loading or enabling Mozc in Windows
#856
Comments
coooooooozy
pushed a commit
to coooooooozy/mozc
that referenced
this issue
Dec 29, 2023
With google/cctz#242, absl::LocalTimeZone() now calls RoInitialize(RO_INIT_MULTITHREADED), which may conflict with CUAS' COM apartment handling when called from Mozc's TIP DLLs. Note that in the current code ClockImpl::timezone_ is always overridden with absl::FixedTimeZone on ChromeOS and Windows, which means that calling absl::LocalTimeZone() on such platforms. Closes google#856. PiperOrigin-RevId: 593601170
yukawa
added a commit
to yukawa/mozc
that referenced
this issue
Oct 14, 2024
This commit affects only mozc_tip{32,64}.dll build with debug mode. There must be no behavior change in release build. Currently 'Mozc_tsf_ui.log' is created only when NDEBUG is defined. With this commit we stop creating it even when NDEBUG is defined. This addresses a crash issue in debug builds discussed in google#1077. See google#856 about why absl::LocalTimeZone cannot be used in Windows right now. Note that this commit may also help us diagnose google#1076, where Windows.Storage.OneCore.dll looks to be intercepting certain Win32 file I/O API calls in AppContainer processes then trigger RoInitialize as needed. Creating 'Mozc_tsf_ui.log' only in debug builds can make our debugging more complicated.
yukawa
added a commit
to yukawa/mozc
that referenced
this issue
Oct 14, 2024
This commit affects only mozc_tip{32,64}.dll build with debug mode. There must be no behavior change in release build. Currently 'Mozc_tsf_ui.log' is created only when NDEBUG is defined. With this commit we stop creating it even when NDEBUG is defined. This addresses a crash issue in debug builds discussed in google#1077. See google#856 about why absl::LocalTimeZone cannot be used in Windows right now. Note that this commit may also help us diagnose google#1076, where Windows.Storage.OneCore.dll looks to be intercepting certain Win32 file I/O API calls in AppContainer processes then trigger RoInitialize as needed. Creating 'Mozc_tsf_ui.log' only in debug builds can make our debugging more complicated.
yukawa
added a commit
to yukawa/mozc
that referenced
this issue
Oct 14, 2024
This attemps to avoid the deadlock issue discussed in google#1076. From what I can observe on Windows 11 23H2 (22631.4317), it looks to be dangerous for us to call File I/O Win32 APIs such as CreateFile from AppContainer processes unless we are confident that the target file/dir is accessible to the AppContainer process. When such an access is not allowed at the ACL level, the debugger shows that Windows.Storage.OneCore.dll tries to forward it to a broker process (e.g. via BrokeredCreateFile2). The issue is that Windows.Storage.OneCore.dll is a WinRT component, which means that just calling CreateFile() results in dyanamically invoking RoInitialize() internally, which can confuse TSF runtime as we have already seen in google#856. To summarize, the safest option is to ensure that the target file/dir is always accessible to AppContainer so that Windows.Storage.OneCore.dll will not be involved if the file/dir is opened with CreateFile() from MozcTip{32,64}.dll. If this commit is not sufficient to address google#1076, we then need to take care of other File I/O APIs such as CreateDirectoryW() and GetFileAttributes().
yukawa
added a commit
to yukawa/mozc
that referenced
this issue
Oct 14, 2024
This attempts to avoid the deadlock issue discussed in google#1076. From what I can observe on Windows 11 23H2 (22631.4317), it looks to be dangerous for us to call File I/O Win32 APIs such as CreateFile from AppContainer processes unless we are confident that the target file/dir is accessible to the AppContainer process. When such an access is not allowed at the ACL level, the debugger shows that Windows.Storage.OneCore.dll tries to forward it to a broker process (e.g. via BrokeredCreateFile2). The issue is that Windows.Storage.OneCore.dll is a WinRT component, which means that just calling CreateFile() results in dynamically invoking RoInitialize() internally, which can confuse TSF runtime as we have already seen in google#856. To summarize, the safest option is to ensure that the target file/dir is always accessible to AppContainer so that Windows.Storage.OneCore.dll will not be involved if the file/dir is opened with CreateFile() from MozcTip{32,64}.dll. If this commit is not sufficient to address google#1076, we then need to take care of other File I/O APIs such as CreateDirectoryW() and GetFileAttributes().
hiroyuki-komatsu
pushed a commit
that referenced
this issue
Oct 15, 2024
This commit affects only mozc_tip{32,64}.dll build with debug mode. There must be no behavior change in release build. Currently 'Mozc_tsf_ui.log' is created only when NDEBUG is defined. With this commit we stop creating it even when NDEBUG is defined. This addresses a crash issue in debug builds discussed in #1077. See #856 about why absl::LocalTimeZone cannot be used in Windows right now. Note that this commit may also help us diagnose #1076, where Windows.Storage.OneCore.dll looks to be intercepting certain Win32 file I/O API calls in AppContainer processes then trigger RoInitialize as needed. Creating 'Mozc_tsf_ui.log' only in debug builds can make our debugging more complicated. PiperOrigin-RevId: 686025719
I havent' used |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Description
When an application that does not explicitly call
CoInitialize
try to load Mozc's dll (or the user tries to enable Mozc), the application may crash due to system's unexpectedly unloading Mozc's dll.Steps to reproduce
Steps to reproduce the behavior:
Expected behavior
The test app does not crash at the step 3.
Actual behavior
The test app crashes at the step 3.
Version or commit-id
68fea8a
2.29.5288.100 and later versions are considered to be affected.
Environment
Additional context
What's actually happening
CoInitialize
with also installingCoRegisterInitializeSpy
, because TSF is built on top of COM.mozc::Logging::InitLogStream
eventually triggersabsl::LocalTimeZone
, which now callsRoInitialize(RO_INIT_MULTITHREADED)
.RoInitialize(RO_INIT_MULTITHREADED)
gets called throughCoRegisterInitializeSpy
and tries to clean up its default COM apartment by explicitly callingCoUninitialize
.CoUninitialize
forcefully unloads Mozc's TIP DLLs (even if Mozc'sDllCanUnloadNow
returnedS_FALSE
), whileTipTextServiceImpl::ActivateEx
is still in the call stack.Here is the callstack when CUAS internally calls
CoInitialize
at the step 1.Here is the relevant code at the step 2.
mozc/src/win32/tip/tip_text_service.cc
Lines 512 to 513 in 68fea8a
Here is the call stack at the step 3.
The code in question originates from google/cctz#242, and Mozc started using it when updating
abseil-cpp
to 20230802.1 LTS for #841 with 6c920a9.The text was updated successfully, but these errors were encountered: