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

[MonoAPI] embedding API testing and API headers quality of life improvements #64456

Closed
3 tasks done
lambdageek opened this issue Jan 28, 2022 · 3 comments
Closed
3 tasks done
Assignees
Labels
area-VM-meta-mono tracking This issue is tracking the completion of other related issues.
Milestone

Comments

@lambdageek
Copy link
Member

lambdageek commented Jan 28, 2022

The current mono public embedding API headers are awkward to work with both for runtime developers and for embedding hosts that wish to dynamically load the runtime:

  • The API headers are in the same directory as the Mono implementation and the internal Mono headers, making it difficult to understand which headers constitute the public API surface without reading the cmake files. Additionally, other runtimes implementing the mono embedding API cannot easily share the headers.
  • The API headers declare mono API functions with MONO_API return_type function_name (args...), which is reasonable for embedding hosts that link to the runtime at compile time, but are not suitable for hosts that use late binding (dlopen+dlsym) to access the mono symbols. We should reorganize the headers to support late binding. (One idea: define API functions as MONO_API(return_type, function_name, (args...)) and allow users of the headers to re-define the MONO_API macro and include the header multiple times. That would allow C compile-time "code generation" of typedefs and dlsym boilerplate.
  • One client of the new headers would be a testsuite of the embedding API. Previously in github.com/mono/mono, the tests (src/mono/mono/tests/libtest.c) had to duplicate the API function signatures just to declare the function pointers storing the dynamically loaded symbols.

Tasks:

  • move public API headers to src/native/include/mono-2.0 (ie: src/native/include/mono-2.0/mono/metadata/object.h src/native/include/mono-2.0//mono/mini/jit.h etc) [build] Move Mono API headers under src/native/public #64569
  • Define MONO_API_FUNCTION(return_type,name,args_sig) and factor the public headers into "implementation details" headers; move type definitions to separate details headers from function definitions; allow the function definitions headers to be included multiple times and to re-define MONO_API_FUNCTION (r,n,a) [MonoAPI] Split type and function headers, add MONO_API_FUNCTION macro #65446
  • Move the libtest.c and existing embedding API testing code from src/mono/mono/tests to src/tests/Interop/MonoAPI and run the tests on desktop mono configurations. Includes switching the tests to use the new MONO_API_FUNCTION late binding approach. [tests] Add MonoApi runtime tests #65221
@dotnet-issue-labeler dotnet-issue-labeler bot added area-VM-meta-mono untriaged New issue has not been triaged by the area owner labels Jan 28, 2022
@lambdageek
Copy link
Member Author

/cc @joncham

@lambdageek lambdageek added this to the 7.0.0 milestone Jan 28, 2022
@lambdageek lambdageek added tracking This issue is tracking the completion of other related issues. and removed untriaged New issue has not been triaged by the area owner labels Jan 28, 2022
@lambdageek lambdageek self-assigned this Jan 28, 2022
lambdageek added a commit that referenced this issue Feb 15, 2022
Contributes to #64456 

1. Separate the public API headers from the mono implementation
2. Provide a foundation for refactoring the headers (as outlined in #64456) to be more useful for late binding scenarios

* Move public mono headers to src/native/public

* fix mono dbgshim and mscordbi builds

* Pass --nativedir=src/native to offsets-tool.py

* fix wasm build

* fix windows build

* Add a brief README for the public directory
@lambdageek
Copy link
Member Author

related work: Unity-Technologies#4

lambdageek added a commit that referenced this issue Mar 4, 2022
Contributes to #64456

Create a new directory src/tests/Interop/MonoAPI for tests that use the mono embedding API.

Move the mono libtest.c native library along with managed tests: InstallEHCallback.cs, PInvokeDetach.cs and Thunks.cs to src/tests/Interop/MonoAPI/...

The native library (now called mono-embedding-api-test.c) builds on all platforms where we build native support libraries for the runtime tests.

The managed tests only run on desktop mono configurations for now.


* copy libtest.c from mono to src/tests and add cmake builds

* Make it build with the monoapi project

* remove mono-compiler.h dependency

* Remove dependency on gmodule.h

   Had to copy the w32_find_symbol code that searches for a symbol in every module.

* Remove test library eglib dependency

   Delete some unrunnable tests.

   Copy some basic eglib utilities into the test.

* Move native embedding API test lib to Interop/MonoAPI/Native

* Move install_eh_callback test to src/tests/Interop/MonoAPI/MonoMono

* add issues.targets excludes for MonoAPI tests

  Not expected to run on coreclr or mono on wasm or mobile, for now.

* Add PInvokeDetach MonoAPI test

* Return exit code 100 on success

* Change test class names to be unique

* fix win32 and gcc builds

* simplify managed code

* put all the MonoMono tests in a single directory

* Use the new MONO_API_FUNCTION monoapi headers

* compile the native library as C, not C++, same as libtest.c in the past

* delete many unused test functions

* delete unused utilities

* Add Thunks.cs tests

* Use a common setup method to probe for symbols

* simplify the native test library

* Make the PInvokeDetach native code sleep less

* add MIT banner

* skip Thunks test on AOT

   In the mono/mono repo that test was not expected to work with AOT
@lambdageek
Copy link
Member Author

The foundational work is finished. Follow-up work includes #66194, as well as writing additional tests that validate the embedding API calls used by popular embedders such as xamarin-macios or xamarin-android

@ghost ghost locked as resolved and limited conversation to collaborators Apr 3, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-VM-meta-mono tracking This issue is tracking the completion of other related issues.
Projects
None yet
Development

No branches or pull requests

1 participant