Skip to content
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

Segfault with WinRT hosting #643

Closed
timsneath opened this issue Jan 4, 2023 · 4 comments · Fixed by #642
Closed

Segfault with WinRT hosting #643

timsneath opened this issue Jan 4, 2023 · 4 comments · Fixed by #642

Comments

@timsneath
Copy link
Contributor

timsneath commented Jan 4, 2023

This was a segfault, where Dart just quits without warning, rather than a process exit as in #463. The last displayed test attempt was IReference<Int64>. Until the changes I made here, Dart tests ran with the default concurrency settings (two threads), so there's a 1 in 2 chance that it was this specific test that failed. But both tests running would have been from that test file, I think.

I came across this today. You should be able to reproduce this with the following cmd script:

@echo off

rem Run the dart test command 30 times
for /l %%i in (1,1,30) DO (
  echo Run: %%i
  call dart test test/winrt_calendar_test.dart test/winrt_collections_test.dart test/winrt_ireference_test.dart
)

(You may need to run this script several times 😄)

While running this script, you should see either a segfault or an error like this in some random test:

  Error 0x80080008: Object server is stopping when OLE service contacts it
  package:win32/src/winrt_helpers.dart 140:21                ActivateClass
  package:win32/src/winrt/globalization/calendar.dart 37:15  new Calendar
  test\winrt_calendar_test.dart 18:18                        main.<fn>

I think this is caused by calls to the winrtUninitialize() helper function which calls RoUninitialize(). I can't reproduce this if I comment out the call to RoUninitialize() in the winrtUninitialize().

I have looked at the other WinRT projections to see if they call RoUninitialize() and I can confirm that they don't. In fact, they don't call RoInitialize() either. They support automatic initialization of the Windows Runtime by using CoIncrementMTAUsage API.

Here is the relevant snippet from cppwinrt:
https://github.com/microsoft/cppwinrt/blob/fe304096fa30583f3cc2ebfdbf564d2b4081e2ad/strings/base_activation.h#L37-L52

And this is from windows-rs:
https://github.com/microsoft/windows-rs/blob/a8b3b3841ae5f966dd5f86397a0a81eaa26470b6/crates/libs/windows/src/core/factory_cache.rs#L56-L73

Basically, if the call to RoGetActivationFactory fails with the error code CO_E_NOTINITIALIZED, they initialize a Multi-Threaded Apartment (MTA) on the current thread by calling CoIncrementMTAUsage() first and then try again.

I think we should do the same, Created #642 for this. Curious to hear your thoughts.

Note that I forgot to mention in #623, before we make NativeFinalizer call IUnknown's release() automatically on objects we would need to stop calling winrtUninitialize() because, by the time NativeFinalizer callbacks run, the WinRT would be uninitialized which would crash the app (I have experienced this multiple times while working on #623). So, even if we didn't have this problem, we would have to stop using winrtUninitialize at some point.

Originally posted by @halildurmus in #630 (comment)

@timsneath
Copy link
Contributor Author

timsneath commented Jan 4, 2023

I'm surprised by this error message.

I wonder if I can nerd-snipe @kennykerr into sharing some thoughts here? He's written a great article on this (cached here, his home page seems to be down https://cc.bingj.com/cache.aspx?q=CoIncrementMTAUsage+vs+Roinitialize&d=4841149887960635&mkt=en-US&setlang=en-US&w=AsmFBigDdQ-ZgFRX0qBS_Au0frYV0p1i). Unfortunately, it tantalizingly defers explanation of RoInitialize vs CoIncrementMTAUsage:

The differences between these three functions (and others) is not insignificant, but not important for today’s topic.

@kennykerr
Copy link

Hey Tim, well wordpress completely destroyed my blog so I'm left trying to figure out how to set up a static blog from markdown but its proving harder than I'd like. 😊

Anyway, the differences between the various init/uninit functions aren't relevant outside a very small niche of apps and app models that are largely irrelevant today. I'd just do what cppwinrt and windows-rs do and call CoIncrementMTAUsage on demand. The problem you're seeing is that teardown on Windows is practically impossible to do reliably so when you call CoUninitialize/RoUninitialize you're just asking for unreliability since it may force DLLs to be unloaded that outstanding COM interface pointers are still, well, pointing to. And even if nobody is pointing at them, those DLLs are still unloaded in a non-deterministic order such that any inter-dependent pointers run into the same problem.

The only reliable way to handle this is to call CoIncrementMTAUsage up front and use TerminateProcess (or equivalent) when you're done.

https://twitter.com/jamesmcnellis/status/707650369073995776 😉

@kennykerr
Copy link

Oh, you did ask about the actual difference. I talked about that here: https://web.archive.org/web/20221128052910/https://kennykerr.ca/2018/04/03/cppwinrt-understanding-coroutines-and-the-calling-context/

@timsneath
Copy link
Contributor Author

You remain my hero, @kennykerr! Thank you so much for validating the approach here. And I'm reading your second article now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants