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.

Sign up for GitHub

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

Jump to bottom

Hybrid suspend: mono_add_internal_call is broken #11138

Closed
lambdageek opened this issue Oct 12, 2018 · 1 comment
Closed

Hybrid suspend: mono_add_internal_call is broken #11138

lambdageek opened this issue Oct 12, 2018 · 1 comment
Assignees
@lambdageek
Labels
area-Runtime: Threading
Milestone
2018-08 (5.18.xx)

Comments

@lambdageek
Copy link
Member

  • Embedders can use mono_add_internal_call to register an icall.
  • Hybrid suspend runs icalls in GC Unsafe (cooperative suspended) mode.
  • Embedders don't know that and rely on being able to call blocking operations.

(fixable complication: the runtime also calls mono_add_internal_call - we should redirect those to a new entrypoint)

We should run embedder icalls in GC Safe (preemptively suspended) mode under hybrid suspend.
@lambdageek lambdageek added this to the 2018-08 (5.18.xx) milestone Oct 12, 2018
@lambdageek lambdageek self-assigned this Oct 12, 2018
@lambdageek
Copy link
Member Author

For example XM xamarin_release_managed_ref calls xamarin_framework_peer_lock(); xamarin_framework_peer_unlock ();
https://github.com/xamarin/xamarin-macios/blob/f4dbb6840ae6d908523ae69f5b5a01c75228fc88/runtime/runtime.m#L1998-L1999

This was referenced Oct 12, 2018
Open
Closed
This was referenced Oct 15, 2018
Closed
Closed
@lambdageek lambdageek mentioned this issue Oct 15, 2018
Merged
@lambdageek lambdageek mentioned this issue Oct 15, 2018
Merged
@lambdageek lambdageek mentioned this issue Oct 15, 2018
Merged
lambdageek added a commit to lambdageek/mono that referenced this issue Oct 18, 2018
@lambdageek
[marshal] Emit GC Safe transitions around foreign icalls.
6cff137 
A foreign icall is added using mono_add_internal_call and that is not coop GC
aware.  (The runtime uses mono_add_internal_call_with_flags or
mono_add_internal_call_internal)

Under hybrid suspend, foreign icalls will run in GC Safe mode and transition to
GC Unsafe only when the call back into the runtime or invoke managed code.

Fixes mono#11138
lambdageek added a commit to lambdageek/mono that referenced this issue Oct 18, 2018
@lambdageek
[marshal] Emit GC Safe transitions around foreign icalls.
4bc395c 
A foreign icall is added using mono_add_internal_call and that is not coop GC
aware.  (The runtime uses mono_add_internal_call_with_flags or
mono_add_internal_call_internal)

Under hybrid suspend, foreign icalls will run in GC Safe mode and transition to
GC Unsafe only when the call back into the runtime or invoke managed code.

Fixes mono#11138
lambdageek added a commit to lambdageek/mono that referenced this issue Oct 19, 2018
@lambdageek
[marshal] Emit GC Safe transitions around foreign icalls.
0029c91 
A foreign icall is added using mono_add_internal_call and that is not coop GC
aware.  (The runtime uses mono_add_internal_call_with_flags or
mono_add_internal_call_internal)

Under hybrid suspend, foreign icalls will run in GC Safe mode and transition to
GC Unsafe only when the call back into the runtime or invoke managed code.

Fixes mono#11138
marek-safar pushed a commit that referenced this issue Oct 22, 2018
@lambdageek @marek-safar
[2018-08][marshal] Emit GC Safe transitions around foreign internal c…
5756ba4 
…alls. (#11180)

* [threads] Make mono_add_internal_call external only

The runtime should use mono_add_internal_call_internal or
mono_add_internal_call_with_flags.

The thread suspend mechanism needs to know if an icall is added via the legacy
mono_add_internal_call API (which is not coop-aware, and so the registered
icalls must run in GC Safe mode), or if it is added by the runtime or by a
cooperative client (currently either the profiler or the System.Native PAL)
which knows to add GC transitions and safepoints and not to block indefinitely.

* [marshal] Factor out GCSafeTransitionBuilder

Make a local builder for creating the GC Safe transition calls for a method wraper.

* [sgen] Add coop GC transitions in mono_gc_toggleref_add

Also mark it external only. It's not used inside the runtime.

* [marshal] Emit GC Safe transitions around foreign icalls.

A foreign icall is added using mono_add_internal_call and that is not coop GC
aware.  (The runtime uses mono_add_internal_call_with_flags or
mono_add_internal_call_internal)

Under hybrid suspend, foreign icalls will run in GC Safe mode and transition to
GC Unsafe only when the call back into the runtime or invoke managed code.

Fixes #11138

* [runtime] Add new API call mono_dangerous_add_raw_internal_call

Under hybrid suspend, this adds an icall that is assumed to run in GC Unsafe
mode.  As such it has additional requirements for correct operation: it must
not run loops without periodically polling the runtime, and it must not perform
blocking operations such as blocking I/O or taking locks without manually
switching to GC Safe mode.
marek-safar pushed a commit that referenced this issue Oct 22, 2018
@lambdageek @marek-safar
[2018-10][marshal] Emit GC Safe transitions around foreign internal c…
bbcc3eb 
…alls (#11184)

* [threads] Make mono_add_internal_call external only

The runtime should use mono_add_internal_call_internal or
mono_add_internal_call_with_flags.

The thread suspend mechanism needs to know if an icall is added via the legacy
mono_add_internal_call API (which is not coop-aware, and so the registered
icalls must run in GC Safe mode), or if it is added by the runtime or by a
cooperative client (currently either the profiler or the System.Native PAL)
which knows to add GC transitions and safepoints and not to block indefinitely.

* [marshal] Factor out GCSafeTransitionBuilder

Make a local builder for creating the GC Safe transition calls for a method wraper.

* [sgen] Add coop GC transitions in mono_gc_toggleref_add

Also mark it external only. It's not used inside the runtime.

* [marshal] Emit GC Safe transitions around foreign icalls.

A foreign icall is added using mono_add_internal_call and that is not coop GC
aware.  (The runtime uses mono_add_internal_call_with_flags or
mono_add_internal_call_internal)

Under hybrid suspend, foreign icalls will run in GC Safe mode and transition to
GC Unsafe only when the call back into the runtime or invoke managed code.

Fixes #11138

* [cxx] function type overloads for mono_add_internal_call_internal

and mono_add_internal_call_with_flags

* [runtime] Add new API call mono_dangerous_add_raw_internal_call

Under hybrid suspend, this adds an icall that is assumed to run in GC Unsafe
mode.  As such it has additional requirements for correct operation: it must
not run loops without periodically polling the runtime, and it must not perform
blocking operations such as blocking I/O or taking locks without manually
switching to GC Safe mode.
@lambdageek lambdageek mentioned this issue Oct 23, 2018
Merged
@lambdageek lambdageek mentioned this issue Oct 26, 2018
Closed
baulig pushed a commit that referenced this issue Oct 29, 2018
@lambdageek
[2018-10][marshal] Emit GC Safe transitions around foreign internal c…
6d72ff5 
…alls (#11184)

* [threads] Make mono_add_internal_call external only

The runtime should use mono_add_internal_call_internal or
mono_add_internal_call_with_flags.

The thread suspend mechanism needs to know if an icall is added via the legacy
mono_add_internal_call API (which is not coop-aware, and so the registered
icalls must run in GC Safe mode), or if it is added by the runtime or by a
cooperative client (currently either the profiler or the System.Native PAL)
which knows to add GC transitions and safepoints and not to block indefinitely.

* [marshal] Factor out GCSafeTransitionBuilder

Make a local builder for creating the GC Safe transition calls for a method wraper.

* [sgen] Add coop GC transitions in mono_gc_toggleref_add

Also mark it external only. It's not used inside the runtime.

* [marshal] Emit GC Safe transitions around foreign icalls.

A foreign icall is added using mono_add_internal_call and that is not coop GC
aware.  (The runtime uses mono_add_internal_call_with_flags or
mono_add_internal_call_internal)

Under hybrid suspend, foreign icalls will run in GC Safe mode and transition to
GC Unsafe only when the call back into the runtime or invoke managed code.

Fixes #11138

* [cxx] function type overloads for mono_add_internal_call_internal

and mono_add_internal_call_with_flags

* [runtime] Add new API call mono_dangerous_add_raw_internal_call

Under hybrid suspend, this adds an icall that is assumed to run in GC Unsafe
mode.  As such it has additional requirements for correct operation: it must
not run loops without periodically polling the runtime, and it must not perform
blocking operations such as blocking I/O or taking locks without manually
switching to GC Safe mode.

(cherry picked from commit bbcc3eb)
baulig pushed a commit to baulig/mono that referenced this issue Oct 31, 2018
@lambdageek
[2018-10][marshal] Emit GC Safe transitions around foreign internal c…
b4c4513 
…alls (mono#11184)

* [threads] Make mono_add_internal_call external only

The runtime should use mono_add_internal_call_internal or
mono_add_internal_call_with_flags.

The thread suspend mechanism needs to know if an icall is added via the legacy
mono_add_internal_call API (which is not coop-aware, and so the registered
icalls must run in GC Safe mode), or if it is added by the runtime or by a
cooperative client (currently either the profiler or the System.Native PAL)
which knows to add GC transitions and safepoints and not to block indefinitely.

* [marshal] Factor out GCSafeTransitionBuilder

Make a local builder for creating the GC Safe transition calls for a method wraper.

* [sgen] Add coop GC transitions in mono_gc_toggleref_add

Also mark it external only. It's not used inside the runtime.

* [marshal] Emit GC Safe transitions around foreign icalls.

A foreign icall is added using mono_add_internal_call and that is not coop GC
aware.  (The runtime uses mono_add_internal_call_with_flags or
mono_add_internal_call_internal)

Under hybrid suspend, foreign icalls will run in GC Safe mode and transition to
GC Unsafe only when the call back into the runtime or invoke managed code.

Fixes mono#11138

* [cxx] function type overloads for mono_add_internal_call_internal

and mono_add_internal_call_with_flags

* [runtime] Add new API call mono_dangerous_add_raw_internal_call

Under hybrid suspend, this adds an icall that is assumed to run in GC Unsafe
mode.  As such it has additional requirements for correct operation: it must
not run loops without periodically polling the runtime, and it must not perform
blocking operations such as blocking I/O or taking locks without manually
switching to GC Safe mode.

(cherry picked from commit bbcc3eb)
jonpryor pushed a commit to dotnet/android that referenced this issue Dec 6, 2018
@directhex @jonpryor
Bump to mono/mono@23f2024a (#2048)
3897e86 
Bumps to mono/api-snapshot@b99fc87c.
Bumps to mono/bockbuild@5af573e7.
Bumps to mono/boringssl@41221b45.
Bumps to mono/corefx@23d0b584.
Bumps to mono/corert@af496fc1.
Bumps to dotnet/linker@7af03ce0.
Bumps to mono/NUnitLite@00e259a4.
Bumps to mono/reference-assemblies@93258263.
Bumps to mono/roslyn-binaries@249709fa.
Bumps to mono/xunit-binaries@bb583470.

	$ git diff --shortstat b63e5378..23f2024a      # mono 
	 1630 files changed, 50926 insertions(+), 92212 deletions(-)

Fixes: mono/mono#6352
Fixes: mono/mono#6947
Fixes: mono/mono#6992
Fixes: mono/mono#7615
Fixes: mono/mono#8340
Fixes: mono/mono#8407
Fixes: mono/mono#8575
Fixes: mono/mono#8627
Fixes: mono/mono#8707
Fixes: mono/mono#8766
Fixes: mono/mono#8848
Fixes: mono/mono#8866
Fixes: mono/mono#8935
Fixes: mono/mono#9010
Fixes: mono/mono#9023
Fixes: mono/mono#9031
Fixes: mono/mono#9033
Fixes: mono/mono#9106
Fixes: mono/mono#9109
Fixes: mono/mono#9155
Fixes: mono/mono#9179
Fixes: mono/mono#9232
Fixes: mono/mono#9234
Fixes: mono/mono#9262
Fixes: mono/mono#9277
Fixes: mono/mono#9292
Fixes: mono/mono#9318
Fixes: mono/mono#9318
Fixes: mono/mono#9332
Fixes: mono/mono#9407
Fixes: mono/mono#9421
Fixes: mono/mono#9505
Fixes: mono/mono#9542
Fixes: mono/mono#9581
Fixes: mono/mono#9623
Fixes: mono/mono#9684
Fixes: mono/mono#9750
Fixes: mono/mono#9753
Fixes: mono/mono#9772
Fixes: mono/mono#9839
Fixes: mono/mono#9869
Fixes: mono/mono#9921
Fixes: mono/mono#9943
Fixes: mono/mono#9947
Fixes: mono/mono#9973
Fixes: mono/mono#9996
Fixes: mono/mono#10000
Fixes: mono/mono#10031
Fixes: mono/mono#10035
Fixes: mono/mono#10227
Fixes: mono/mono#10243
Fixes: mono/mono#10303
Fixes: mono/mono#10448
Fixes: mono/mono#10483
Fixes: mono/mono#10488
Fixes: mono/mono#10863
Fixes: mono/mono#11123
Fixes: mono/mono#11138
Fixes? mono/mono#11146
Fixes: mono/mono#11202
Fixes: mono/mono#11378
Fixes: mono/mono#11479
Fixes: mono/mono#11613
Fixes: #1951
Fixes: xamarin/xamarin-macios#4347
Fixes: xamarin/xamarin-macios#4617
Fixes: xamarin/xamarin-macios#4984
jonpryor pushed a commit to dotnet/android that referenced this issue Apr 24, 2019
@directhex @jonpryor
Bump to mono/mono@e66c7667 (#2701)
b8c30f9 
Bumps to mono/api-snapshot@ae01378
Bumps to mono/reference-assemblies@e5173a5
Bumps to mono/bockbuild@d30329d
Bumps to mono/boringssl@3d87996
Bumps to mono/corefx@72f7d76
Bumps to mono/corert@1b7d4a1
Bumps to mono/helix-binaries@7e893ea
Bumps to mono/illinker-test-assets@f21ff68
Bumps to dotnet/linker@13d864e
Bumps to mono/llvm@1aaaaa5 [mono]
Bumps to mono/llvm@2c2cffe [xamarin-android]
Bumps to mono/NUnitLite@0029561
Bumps to mono/roslyn-binaries@0bbc9b4
Bumps to mono/xunit-binaries@8f6e62e

	$ git diff --shortstat 886c4901..e66c7667      # mono
        3597 files changed, 350850 insertions(+), 91128 deletions(-)
	$ git diff --shortstat 349752c464c5fc93b32e7d45825f2890c85c8b7d..2c2cffedf01e0fe266b9aaad2c2563e05b750ff4
	 240 files changed, 18562 insertions(+), 6581 deletions(-)

Context: https://github.com/dotnet/coreclr/issues/22046

Fixes: CVE 2018-8292 on macOS
Fixes: http://work.devdiv.io/737323
Fixes: https://github.com/dotnet/corefx/issues/33965
Fixes: dotnet/standard#642
Fixes: mono/mono#6997
Fixes: mono/mono#7326
Fixes: mono/mono#7517
Fixes: mono/mono#7750
Fixes: mono/mono#7859
Fixes: mono/mono#8360
Fixes: mono/mono#8460
Fixes: mono/mono#8766
Fixes: mono/mono#8922
Fixes: mono/mono#9418
Fixes: mono/mono#9507
Fixes: mono/mono#9951
Fixes: mono/mono#10024
Fixes: mono/mono#10030
Fixes: mono/mono#10038
Fixes: mono/mono#10448
Fixes: mono/mono#10735
Fixes: mono/mono#10735
Fixes: mono/mono#10737
Fixes: mono/mono#10743
Fixes: mono/mono#10834
Fixes: mono/mono#10837
Fixes: mono/mono#10838
Fixes: mono/mono#10863
Fixes: mono/mono#10945
Fixes: mono/mono#11020
Fixes: mono/mono#11021
Fixes: mono/mono#11021
Fixes: mono/mono#11049
Fixes: mono/mono#11091
Fixes: mono/mono#11095
Fixes: mono/mono#11123
Fixes: mono/mono#11138
Fixes: mono/mono#11146
Fixes: mono/mono#11202
Fixes: mono/mono#11214
Fixes: mono/mono#11317
Fixes: mono/mono#11326
Fixes: mono/mono#11378
Fixes: mono/mono#11385
Fixes: mono/mono#11478
Fixes: mono/mono#11479
Fixes: mono/mono#11488
Fixes: mono/mono#11489
Fixes: mono/mono#11527
Fixes: mono/mono#11529
Fixes: mono/mono#11596
Fixes: mono/mono#11603
Fixes: mono/mono#11613
Fixes: mono/mono#11623
Fixes: mono/mono#11663
Fixes: mono/mono#11681
Fixes: mono/mono#11684
Fixes: mono/mono#11693
Fixes: mono/mono#11697
Fixes: mono/mono#11779
Fixes: mono/mono#11809
Fixes: mono/mono#11858
Fixes: mono/mono#11895
Fixes: mono/mono#11898
Fixes: mono/mono#11898
Fixes: mono/mono#11965
Fixes: mono/mono#12182
Fixes: mono/mono#12193
Fixes: mono/mono#12218
Fixes: mono/mono#12235
Fixes: mono/mono#12263
Fixes: mono/mono#12307
Fixes: mono/mono#12331
Fixes: mono/mono#12362
Fixes: mono/mono#12374
Fixes: mono/mono#12402
Fixes: mono/mono#12421
Fixes: mono/mono#12461
Fixes: mono/mono#12479
Fixes: mono/mono#12479
Fixes: mono/mono#12552
Fixes: mono/mono#12603
Fixes: mono/mono#12747
Fixes: mono/mono#12831
Fixes: mono/mono#12843
Fixes: mono/mono#12881
Fixes: mono/mono#13030
Fixes: mono/mono#13284
Fixes: mono/mono#13297
Fixes: mono/mono#13455
Fixes: mono/mono#13460
Fixes: mono/mono#13478
Fixes: mono/mono#13479
Fixes: mono/mono#13522
Fixes: mono/mono#13607
Fixes: mono/mono#13610
Fixes: mono/mono#13610
Fixes: mono/mono#13639
Fixes: mono/mono#13672
Fixes: mono/mono#13834
Fixes: mono/mono#13878
Fixes: mono/mono#6352
Fixes: mono/monodevelop#6898
Fixes: xamarin/maccore#1069
Fixes: xamarin/maccore#1407
Fixes: xamarin/maccore#604
Fixes: xamarin/xamarin-macios#4984
Fixes: xamarin/xamarin-macios#5289
Fixes: xamarin/xamarin-macios#5363
Fixes: xamarin/xamarin-macios#5381
Fixes: https://issuetracker.unity3d.com/issues/editor-crashes-with-g-logv-when-entering-play-mode-with-active-flowcanvas-script
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@lambdageek lambdageek

Labels
area-Runtime: Threading
Projects
None yet
Milestone
2018-08 (5.18.xx)
Development

No branches or pull requests
1 participant
@lambdageek