Skip to content

Conversation

@lambdageek
Copy link
Member

@lambdageek lambdageek commented Mar 18, 2020

See #33633 and https://github.com/xamarin/xamarin-macios/blob/master/runtime/exports.t4

  • mono_install_load_aot_data_hook was already a MONO_API, but it was not in a public header.
    It is added to mono/jit/mono-private-unstable.h
  • 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). It is now a public Mono API function.
  • mono_trace_init was a MONO_API function in a non-public header that was already used by embedders that need to set up logger hooks. It is now not necessary to call this before calling the logger functions to set up logger hooks such as mono_trace_set_log_handler

@EgorBo
Copy link
Member

EgorBo commented Mar 18, 2020

heh, I was about to file an issue 🙂

@EgorBo
Copy link
Member

EgorBo commented Mar 18, 2020

since #33633 is already merged could you please update runtime.m ? https://github.com/dotnet/runtime/blob/master/src/mono/netcore/sample/iOS/runtime.m#L15-L20

@lambdageek
Copy link
Member Author

Verified that the ios sample builds in Xcode.

@vargaz
Copy link
Contributor

vargaz commented Mar 19, 2020

So we didn't put them in public headers to avoid having to avoid users depending on them and thus having to support them forever.

@akoeplinger
Copy link
Member

@vargaz you could argue that with XI relying on them we're already on that path. Do you see any of these APIs as particularly problematic?

@lambdageek lambdageek changed the title [mono] Make some API functions public wip: [mono] Make some API functions public Mar 20, 2020
@lambdageek
Copy link
Member Author

lambdageek commented Mar 20, 2020

new plan:

  1. mono_trace_init is going to stay private. instead i'm going to update mono_trace_set_log_handler and other logging APIs to implicitly initialize the logger so that the iOS sample (and XI) don't need to call it anymore.
  2. mono_gc_init_finalizer_thread will become a public API (there's no other way to work with a --with-lazy-thread-creation runtime)
  3. Move mono_install_load_aot_data_hook to the new mono/jit/mono-private-unstable.h header from [mono] Add headers for unstable APIs #33869

lambdageek and others added 5 commits March 23, 2020 21:10
It's already a MONO_API and it is used, for example, by Xamarin.iOS, but it
wasn't in a public header.

Mark it external only.  There are no uses of it inside the runtiem
It was already used by Xamarin.iOS but wasn't in a public header and required
compiling with --disable-visibility-hidden in order to dlsym the symbol.

If --with-lazy-gc-thread-creation is used, embedders must call this function to
create the finalizer therad.  Otherwise the function does nothing and the
runtime will create the finalizer thread automatically.
It was already marked as MONO_API, but it was not in a public header.
It is used by embedders such as Xamarin.iOS
@lambdageek lambdageek force-pushed the add-xi-nonpublic-apis branch from 93a416a to c037a3e Compare March 24, 2020 01:29
@lambdageek lambdageek changed the title wip: [mono] Make some API functions public [mono] Make some API functions public and private Mar 24, 2020
@lambdageek lambdageek requested a review from EgorBo March 24, 2020 01:32
@lambdageek
Copy link
Member Author

Changed this PR to make mono_install_load_aot_data_hook an unstable API, and to make mono_trace_init unnecessary.

@EgorBo could you check that the sample still compiles.

Embedders don't need to call mono_trace_init before calling
mono_trace_set_log_handler, mono_trace_set_print_handler or mono_trace_set_printerr_handler
We do not guarantee that this API will be stable
@lambdageek lambdageek force-pushed the add-xi-nonpublic-apis branch from c037a3e to 7dfc6ef Compare March 24, 2020 01:35
@EgorBo
Copy link
Member

EgorBo commented Mar 24, 2020

@lambdageek just checked, compiles and works 👍

@lambdageek lambdageek merged commit c1686da into dotnet:master Mar 24, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 10, 2020
@lambdageek lambdageek deleted the add-xi-nonpublic-apis branch March 7, 2021 21:54
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.

5 participants