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

[mono] Add headers for unstable APIs #33869

Merged
merged 2 commits into from
Mar 23, 2020

Conversation

lambdageek
Copy link
Member

We have functions that are de-facto part of the Mono embedding API that are
used by Xamarin.iOS or Xamarin.Android that are not in a state where we want to
commit to supporting them forever in their current form.

Nonetheless, we would like to make sure that the symbols for these functions
are visible even if we otherwise compile the runtime with
-fvisisibility=hidden, and we would like to discourage declaring the
functions in the embedder's own headers.

The functions that go into the
mono/{utils,metadata,jit}/mono-private-unstable.h headers will all be marked
with MONO_API MONO_RT_EXTERNAL_ONLY but we will not guarantee that the
functions will be there from one release to the next or that they will not
change their signatures or their behaviors.

We have functions that are de-facto part of the Mono embedding API that are
used by Xamarin.iOS or Xamarin.Android that are not in a state where we want to
commit to supporting them forever in their current form.

Nonetheless, we would like to make sure that the symbols for these functions
are visible even if we otherwise compile the runtime with
`-fvisisibility=hidden`, and we would like to discourage declaring the
functions in the embedder's own headers.

The functions that go into the
`mono/{utils,metadata,jit}/mono-private-unstable.h` headers will all be marked
with `MONO_API MONO_RT_EXTERNAL_ONLY` but we will not guarantee that the
functions will be there from one release to the next or that they will not
change their signatures or their behaviors.
@lambdageek
Copy link
Member Author

Attn: @vargaz @migueldeicaza

@lambdageek
Copy link
Member Author

An example where we might want to use this header is for mono_install_load_aot_data_hook and mono_gc_init_finalizer_thread from #33736

From that PR:

  • mono_install_load_aot_data_hook was already a MONO_API, but it was not in a public header. See https://github.com/xamarin/xamarin-macios/blob/master/runtime/exports.t4
  • mono_gc_init_finalizer_thread was used by Xamarin.iOS for a long time without being in any public header. It's one of the reasons we have to compile Mono for XI with --disable-visibility-hidden. Make the function a proper MONO_API unconditionally - but make it do nothing if the runtime is compiled without --with-lazy-thread-creation (the default).

@lambdageek
Copy link
Member Author

FYI @jkotas

@lambdageek

This comment has been minimized.

@lambdageek lambdageek changed the title RFC: [mono] Add headers for unstable APIs [mono] Add headers for unstable APIs Mar 20, 2020
@jaykrell
Copy link
Contributor

Perhaps this set should be extern "C++" (with a C++ wrapper even), so that signature changes are fatal. I understand the implications -- each platform/architecture gets different names.
Alternatively, a policy of changing the name when changing signature.

@lambdageek lambdageek force-pushed the add-mono-unstable-api-headers branch from 0d8fd42 to 34c3ceb Compare March 20, 2020 22:19
@lambdageek
Copy link
Member Author

lambdageek commented Mar 20, 2020

Perhaps this set should be extern "C++" (with a C++ wrapper even), so that signature changes are fatal.

Problem is that some embedders like mono/tests/libtest.c or Xamarin.iOS will use dlsym to get at these symbols so mangling the name to include the types will cause us pain.

Alternatively, a policy of changing the name when changing signature.

yea that's an interesting idea. I would say changing the semantics should also require a name change (though that may be harder to catch). Maybe just bumping some numeric suffic on each function name. Nothing says unstable like mono_zombo_init_37

@jaykrell
Copy link
Contributor

I understand the dlsym part, but that is not insurmountable.
The names are discoverable.
People do use GetProcAddress and dlsym with mangled names.
Some kind of "manual name mangling" is easier but not enforced.

@lambdageek lambdageek merged commit ccada16 into dotnet:master Mar 23, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 10, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants