Skip to content

[threads] Add back mono_threads_attach_tools_thread as a public API#18048

Merged
lambdageek merged 2 commits intomono:masterfrom
lambdageek:fix-add-tools-threads
Dec 6, 2019
Merged

[threads] Add back mono_threads_attach_tools_thread as a public API#18048
lambdageek merged 2 commits intomono:masterfrom
lambdageek:fix-add-tools-threads

Conversation

@lambdageek
Copy link
Member

In a5da7b2
we got rid of "tools" threads internally to the runtime.

However since the API was previously marked with MONO_API it was an internal
API that some embedders depended on.

This PR adds back an (external-only) limited form of tools thread.

The runtime is aware of the Tools thread in that FOREACH_THREAD_* macros will
iterate over them, and the thread has a coop thread state machine. (That is,
mono_thread_info_current() and GC Safe and GC Unsafe transitions all work.)

However the thread is:

  1. Not stopped by the GC
  2. Is not interrupted by profiler sampling.
  3. Does not have a "current domain"
  4. (As a consequence of the above) cannot call managed methods or touch managed
    objects.

Such threads are useful for low-level interaction with the runtime such as
querying metadata, the JIT state and other coordination.

mono_threads_attach_tools_thread should be called no more than once. It should
not be called on a thread that is already attached with mono_thread_atach, and
vice versa.

Addresses #18011

In mono@a5da7b2
we got rid of "tools" threads internally to the runtime.

However since the API was previously marked with MONO_API it was an internal
API that some embedders depended on.

This PR adds back an (external-only) limited form of tools thread.

The runtime is aware of the Tools thread in that FOREACH_THREAD_* macros will
iterate over them, and the thread has a coop thread state machine. (That is,
mono_thread_info_current() and GC Safe and GC Unsafe transitions all work.)

However the thread is:
1. Not stopped by the GC
2. Is not interrupted by profiler sampling.
3. Does not have a "current domain"
4. (As a consequence of the above) cannot call managed methods or touch managed
objects.

Such threads are useful for low-level interaction with the runtime such as
querying metadata, the JIT state and other coordination.

mono_threads_attach_tools_thread should be called no more than once.  It should
not be called on a thread that is already attached with mono_thread_atach, and
vice versa.

Addresses mono#18011
@lambdageek
Copy link
Member Author

This is enough to get the sample from https://github.com/lambdageek/mono-mach-exceptions working.

Another option is to do stuff with mach ports in Mono directly and provide a mach exception hook that an embedder can register. We'd probably have to expose a function like:

  typedef ... (*mono_mach_exception_hook_fn)(...);
  void mono_mach_add_exception_hook (mono_mach_exception_hook_fn fnptr);

where mono_mach_exception_hook_fn would be a signature like that of catch_exception_raise from Mach

@alexrp
Copy link
Contributor

alexrp commented Dec 4, 2019

Is it really a good idea to take a stance of supporting legacy APIs just because they happened to be exported from libmono.so? It's not like mono-threads.h is a public header. Many other symbols (monoeg_..., ves_icall_..., sgen_..., etc) are incidentally exported from libmono.so but that doesn't mean people are supposed to be accessing/using them.

It seems to me that if these APIs are needed by embedders, then they should be properly promoted to an actual public header with the support commitment that goes with that.

@lambdageek
Copy link
Member Author

It seems to me that if these APIs are needed by embedders, then they should be properly promoted to an actual public header with the support commitment that goes with that.

That's true.

@lambdageek
Copy link
Member Author

lambdageek commented Dec 4, 2019

Made it a public API

@lambdageek lambdageek changed the title [utils] Add back mono_threads_attach_tools_thread [threads] Add back mono_threads_attach_tools_thread as a public API Dec 4, 2019
@lambdageek lambdageek marked this pull request as ready for review December 4, 2019 21:20
@lambdageek lambdageek requested a review from vargaz December 4, 2019 21:20
@lambdageek lambdageek force-pushed the fix-add-tools-threads branch from 0e7db0d to a00b5bb Compare December 4, 2019 22:55
@jaykrell
Copy link
Contributor

jaykrell commented Dec 5, 2019

@monojenkins build failed

@lambdageek lambdageek force-pushed the fix-add-tools-threads branch from a00b5bb to c6b6dba Compare December 5, 2019 16:44
@lambdageek
Copy link
Member Author

@monojenkins backport to 2019-12

@lambdageek
Copy link
Member Author

@monojenkins backport to 2019-10

@lambdageek lambdageek merged commit 3dabedd into mono:master Dec 6, 2019
ManickaP pushed a commit to ManickaP/runtime that referenced this pull request Jan 20, 2020
…ono/mono#18048)

* [utils] Add back mono_threads_attach_tools_thread

In https://github.com/mono/mono/commit/mono/mono@a5da7b21f4b6dbc5eaa09c2addee91b84dc1dbd5
we got rid of "tools" threads internally to the runtime.

However since the API was previously marked with MONO_API it was an internal
API that some embedders depended on.

This PR adds back an (external-only) limited form of tools thread.

The runtime is aware of the Tools thread in that FOREACH_THREAD_* macros will
iterate over them, and the thread has a coop thread state machine. (That is,
mono_thread_info_current() and GC Safe and GC Unsafe transitions all work.)

However the thread is:
1. Not stopped by the GC
2. Is not interrupted by profiler sampling.
3. Does not have a "current domain"
4. (As a consequence of the above) cannot call managed methods or touch managed
objects.

Such threads are useful for low-level interaction with the runtime such as
querying metadata, the JIT state and other coordination.

mono_threads_attach_tools_thread should be called no more than once.  It should
not be called on a thread that is already attached with mono_thread_atach, and
vice versa.

Addresses mono/mono#18011

* [threads] Make mono_threads_attach_tools_thread into a public API


Commit migrated from mono/mono@3dabedd
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.

4 participants