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

[threading] embedded mono hangs #16907

Merged
merged 4 commits into from
Sep 25, 2019
Merged

[threading] embedded mono hangs #16907

merged 4 commits into from
Sep 25, 2019

Conversation

thaystg
Copy link
Contributor

@thaystg thaystg commented Sep 18, 2019

When embedded mono is executing a C++ code, it didn't start to execute a C# code, we don't need to suspend the thread to run GC.
So my idea is set the flag MONO_THREAD_INFO_FLAGS_NO_GC when initializes mono embedded and only remove the flag MONO_THREAD_INFO_FLAGS_NO_GC when a mono_runtime_invoke_array or a mono_runtime_invoke is called.

Fixes #16192
Fixes #14725

When embedded mono is executing a C++ code, it didn't start to execute a C# code, we don't need to suspend the thread to run GC.
Fixes mono#16192
Fixes mono#14725
@thaystg thaystg requested a review from lambdageek September 18, 2019 02:56
@thaystg thaystg marked this pull request as ready for review September 23, 2019 18:10
Copy link
Member

@lambdageek lambdageek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Needs some comments. Also a better commit message explaining the embedding scenario that we want to protect.
  2. Could you try also to mark mono_jit_init and mono_jit_init_version as MONO_RT_EXTERNAL_ONLY in jit.h - I think they're unused in the runtime - they're only for embedders.
  3. I would just get rid of the cookie in MONO_ENTER_GC_SAFE_UNBALANCED and delete MONO_EXIT_GC_SAFE_UNBALANCED.

@@ -2754,7 +2754,9 @@ mono_main (int argc, char* argv[])
MonoDomain *
mono_jit_init (const char *file)
{
return mini_init (file, NULL);
MonoDomain *ret = mini_init (file, NULL);
MONO_ENTER_GC_SAFE_UNBALANCED;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need a comment here to explain why we want to leave this thread in GC Safe mode.

@@ -2780,7 +2782,9 @@ mono_jit_init (const char *file)
MonoDomain *
mono_jit_init_version (const char *domain_name, const char *runtime_version)
{
return mini_init (domain_name, runtime_version);
MonoDomain *ret = mini_init (domain_name, runtime_version);
MONO_ENTER_GC_SAFE_UNBALANCED;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also a comment here.

MONO_STACKDATA (__gc_safe_unbalanced_dummy); \
gpointer __gc_safe_unbalanced_cookie = mono_threads_enter_gc_safe_region_unbalanced_internal (&__gc_safe_unbalanced_dummy)

#define MONO_EXIT_GC_SAFE_UNBALANCED \
mono_threads_exit_gc_safe_region_unbalanced_internal (__gc_safe_unbalanced_cookie, &__gc_safe_unbalanced_dummy); \
} while (0)
mono_threads_exit_gc_safe_region_unbalanced_internal (__gc_safe_unbalanced_cookie, &__gc_safe_unbalanced_dummy);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it seems like this macro has almost no value: it won't compile unless you pair it with MONO_ENTER_GC_SAFE_UNBALANCED (because it needs the cookie). But if you use it like that, it's balanced!

@thaystg
Copy link
Contributor Author

thaystg commented Sep 23, 2019

3. MONO_EXIT_GC_SAFE_UNBALANCED

  1. Okay
  2. mono_jit_init_version is used in some tests like: mono/unit-tests/test-mono-string.c and mono/unit-tests/test-mono-callspec.c
  3. Okay

@thaystg
Copy link
Contributor Author

thaystg commented Sep 23, 2019

@monojenkins build failed

3 similar comments
@thaystg
Copy link
Contributor Author

thaystg commented Sep 24, 2019

@monojenkins build failed

@thaystg
Copy link
Contributor Author

thaystg commented Sep 24, 2019

@monojenkins build failed

@thaystg
Copy link
Contributor Author

thaystg commented Sep 24, 2019

@monojenkins build failed

@thaystg
Copy link
Contributor Author

thaystg commented Sep 24, 2019

@monojenkins build failed

1 similar comment
@thaystg
Copy link
Contributor Author

thaystg commented Sep 25, 2019

@monojenkins build failed

@thaystg thaystg merged commit 5b108b5 into mono:master Sep 25, 2019
ManickaP pushed a commit to ManickaP/runtime that referenced this pull request Jan 20, 2020
* [threading] embedded mono hangs

When embedded mono is executing a C++ code, it didn't start to execute a C# code, we don't need to suspend the thread to run GC.
Fixes mono/mono#16192
Fixes mono/mono#14725

* Changing what was discussed with Aleksey during our 1on1

* Changing what was suggested by Aleksey

* Separating mono_init_version that is being used on tests from the embedded ones


Commit migrated from mono/mono@5b108b5
alexischr added a commit to xamarin/xamarin-macios that referenced this pull request Mar 30, 2020
…te transition.

Starting with mono/mono#16907 , the Mono runtime ends in GC-Safe state (mode) after mono_jit_init_version() is called. The monotouch code did not expect that, and calls mono_gc_init_finalizer_thread() which also tries to enter GC-safe mode.

Should fix mono/mono#19372
rolfbjarne pushed a commit to xamarin/xamarin-macios that referenced this pull request Mar 31, 2020
…te transition. (#8242)

Starting with mono/mono#16907 , the Mono runtime ends in GC-Safe state (mode) after mono_jit_init_version() is called. The monotouch code did not expect that, and calls mono_gc_init_finalizer_thread() which also tries to enter GC-safe mode.

Should fix mono/mono#19372
monojenkins pushed a commit to monojenkins/xamarin-macios that referenced this pull request Mar 31, 2020
…te transition.

Starting with mono/mono#16907 , the Mono runtime ends in GC-Safe state (mode) after mono_jit_init_version() is called. The monotouch code did not expect that, and calls mono_gc_init_finalizer_thread() which also tries to enter GC-safe mode.

Should fix mono/mono#19372
monojenkins pushed a commit to monojenkins/xamarin-macios that referenced this pull request Mar 31, 2020
…te transition.

Starting with mono/mono#16907 , the Mono runtime ends in GC-Safe state (mode) after mono_jit_init_version() is called. The monotouch code did not expect that, and calls mono_gc_init_finalizer_thread() which also tries to enter GC-safe mode.

Should fix mono/mono#19372
monojenkins pushed a commit to monojenkins/xamarin-macios that referenced this pull request Mar 31, 2020
…te transition.

Starting with mono/mono#16907 , the Mono runtime ends in GC-Safe state (mode) after mono_jit_init_version() is called. The monotouch code did not expect that, and calls mono_gc_init_finalizer_thread() which also tries to enter GC-safe mode.

Should fix mono/mono#19372
rolfbjarne pushed a commit to xamarin/xamarin-macios that referenced this pull request Mar 31, 2020
…te transition. (#8245)

Starting with mono/mono#16907 , the Mono runtime ends in GC-Safe state (mode) after mono_jit_init_version() is called. The monotouch code did not expect that, and calls mono_gc_init_finalizer_thread() which also tries to enter GC-safe mode.

Should fix mono/mono#19372

Co-authored-by: Alexis Christoforides <alexis@thenull.net>
mandel-macaque pushed a commit to xamarin/xamarin-macios that referenced this pull request Mar 31, 2020
…te transition. (#8248)

Starting with mono/mono#16907 , the Mono runtime ends in GC-Safe state (mode) after mono_jit_init_version() is called. The monotouch code did not expect that, and calls mono_gc_init_finalizer_thread() which also tries to enter GC-safe mode.

Should fix mono/mono#19372

Co-authored-by: Alexis Christoforides <alexis@thenull.net>
alexischr added a commit that referenced this pull request Apr 1, 2020
Starting with #16907 , the runtime ends in GC-Safe state (mode) after mono_jit_init_version() is called. mono_gc_init_finalizer_thread() expects the GC to not already be in safe mode.
alexischr added a commit to alexischr/runtime that referenced this pull request Apr 1, 2020
Starting with mono/mono#16907 , the runtime ends in GC-Safe state (mode) after mono_jit_init_version() is called. mono_gc_init_finalizer_thread() expects the GC to not already be in safe mode.
akoeplinger pushed a commit to dotnet/runtime that referenced this pull request Apr 1, 2020
Starting with mono/mono#16907 , the runtime ends in GC-Safe state (mode) after mono_jit_init_version() is called. mono_gc_init_finalizer_thread() expects the GC to not already be in safe mode.
akoeplinger pushed a commit that referenced this pull request Apr 1, 2020
Starting with #16907 , the runtime ends in GC-Safe state (mode) after mono_jit_init_version() is called. mono_gc_init_finalizer_thread() expects the GC to not already be in safe mode.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Embedded Mono hangs when using native threads mono 5.20 hangs when called from native thread
2 participants