[runtime] AppDomain fixes #5594
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
BrzVlad
requested review from
DavidKarlas,
kumpera,
luhenry and
vargaz
as code owners
luhenry
approved these changes
Sep 15, 2017
luhenry
suggested changes
Sep 15, 2017
mono/metadata/remoting.c
Outdated
| MonoClass *klass = mono_object_get_class ((MonoObject*)exc); | ||
|
|
||
| if (!(mono_class_get_flags (klass) & TYPE_ATTRIBUTE_SERIALIZABLE)) { | ||
| MonoException *ret; |
There was a problem hiding this comment.
Does that mean we are loosing the original exception? Is that the behaviour on .NET?
mono/metadata/object.c
Outdated
| @@ -4587,7 +4587,9 @@ mono_unhandled_exception_checked (MonoObjectHandle exc, MonoError *error) | |||
| MonoObjectHandle current_appdomain_delegate = MONO_HANDLE_NEW (MonoObject, NULL); | |||
|
|
|||
| MonoClass *klass = mono_handle_class (exc); | |||
| if (mono_class_has_parent (klass, mono_defaults.threadabortexception_class)) | |||
| if (klass == mono_defaults.threadabortexception_class || | |||
| (klass == mono_class_get_appdomain_unloaded_exception_class () && | |||
There was a problem hiding this comment.
It seems like it's the correct behaviour according to the doc https://msdn.microsoft.com/en-us/library/system.appdomainunloadedexception(v=vs.110).aspx#Anchor_6 . A link in the commit or the code would be greatly appreciated for further reference
| } | ||
|
|
||
| public static int test_0_unload_reset_abort () { | ||
| AppDomain domain = AppDomain.CreateDomain ("test_0_unload_reset_abort"); |
There was a problem hiding this comment.
Could you also add tests for the expected behaviour in case of AppDomainUnloadedException in the different cases?
BrzVlad
force-pushed
the
fix-domain-abort-hang
branch
2 times, most recently
from
5f4349e
to
0ad9e8e
Compare
…main thread Before we were propagating the abort exception down in the calling domain.
…xception Except for threads started by unmanaged code (like the main thread).
Tests that we can unload domain where the threads reset the abort exception and that this exception is converted to AppDomainUnloadedException in the caller domain.
BrzVlad
force-pushed
the
fix-domain-abort-hang
branch
from
0ad9e8e
to
1384661
Compare
luhenry
approved these changes
Sep 22, 2017
mono/tests/unhandled-exception-9.cs
Outdated
|
|
||
| class Driver | ||
| { | ||
| /* expected exit code: 255 */ |
There was a problem hiding this comment.
If the expected exit code is 255, why is it in the TESTS_UNHANDLED_EXCEPTION_1_SRC with expected exit code of 1?
BrzVlad
force-pushed
the
fix-domain-abort-hang
branch
from
1384661
to
8b959c6
Compare
picenka21
pushed a commit
to picenka21/runtime
that referenced
this pull request
Feb 18, 2022
[runtime] AppDomain fixes Commit migrated from mono/mono@01142a1
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Do it like .NET.