Improve stability on AppDomain reloads. #536
Open
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #525
(at least, it should be much better!)
This PR makes two changes that should significantly decrease the chances of a crash when Unity reloads AppDomains:
CesiumIonSessionImpl
instance will not be destroyed while the network requests it starts are in progress.GCHandle.FromIntPtr
when the AppDomain has been reloaded and the IntPtr corresponds to an object from the old domain. Exceptions are disastrous and often create instability because of Add support for exceptions to Reinterop #18.Unity reloads AppDomains a lot, so anything we can do to more it work more reliably is likely to be a big win for users. Unfortunately, this probably isn't a 100% solution. We really need #18, for one thing. For another, it's not clear to me (even after looking at the Mono source code) whether the "object is from a previous AppDomain" detection is 100% reliable. It might be best effort, and sometimes just give you the "wrong" object instead of throwing an exception.
If that's the case, we might ultimately need to either: 1) implement a way to reliably stop and unload all of the Native world when the AppDomain unloads, or 2) extend our native
ObjectHandle
to record the AppDomain.Id and verify it's still correct before dereferencing. Option (2) is probably more realistic.This PR also updates cesium-native to the current head of main.