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

Objective-C interop design #197

Merged
merged 9 commits into from
May 13, 2021

Conversation

AaronRobinsonMSFT
Copy link
Member

Design document for how .NET can enable Objective-C across multiple .NET runtime implementations.

The design here is represented in the API proposal found at dotnet/runtime#44659.

This design was originally posted at dotnet/runtime#44934.

Co-authored-by: Yaakov <yaakov-h@users.noreply.github.com>
@AaronRobinsonMSFT
Copy link
Member Author

@filipnavara
Copy link
Member

I recently found out that Xamarin also uses Mono profiling API to automatically inject NSAutoreleasePool in every managed thread. Apparently that's covering for a difference between the behaviour of NSThread APIs and the POSIX threads. Is that something that should also be cover by this document or not?

@AaronRobinsonMSFT
Copy link
Member Author

@filipnavara That is definitely related in some ways. That work was integrated into the main branch via dotnet/runtime#47592. I suppose I can add those details here.

@filipnavara
Copy link
Member

filipnavara commented Apr 13, 2021

@AaronRobinsonMSFT That's related but not identical issue. The thread pool needs an explicit autorelease pool that is drained around each work item. What Xamarin does is to setup autorelease pool for every managed thread which is still not implemented on the CoreCLR port.

@AaronRobinsonMSFT
Copy link
Member Author

What Xamarin does is to setup autorelease pool for every managed thread which is still not implemented on the CoreCLR port.

Right, we are only supporting this via the threadpool workitem at least that was my understanding of the plan. The integration with every managed thread hasn't been discussed or considered to my knowledge.

@filipnavara
Copy link
Member

The integration with every managed thread hasn't been discussed or considered to my knowledge.

It's my understanding as well that it was not discussed which is why I am bringing it up. :-) It would be a breaking change with quite a lot of consequences not to create the autorelease pool. At very least I would expect it to be documented so the users who encounter the issue know that they have to insert it explicitly. That may not be always possible when consuming 3rd-party libraries that create their own threads though.

@AaronRobinsonMSFT
Copy link
Member Author

It would be a breaking change with quite a lot of consequences not to create the autorelease pool.

@rolfbjarne or @kouvel May be able to help shed some light on this nuance.

@kouvel
Copy link
Member

kouvel commented Apr 15, 2021

It was my understanding that there would need to be an autorelease pool for every thread that may run managed code, including finalizer thread, tiering background thread, multi-core JIT background thread, etc.

@kouvel
Copy link
Member

kouvel commented Apr 15, 2021

For externally created threads that enter the runtime to run managed code, I'm not sure if an autorelease pool is needed. May depend on how the thread was created, guess it wouldn't hurt to have a pool for that case too.

@rolfbjarne
Copy link
Member

What Xamarin does is to setup autorelease pool for every managed thread which is still not implemented on the CoreCLR port.

Right, we are only supporting this via the threadpool workitem at least that was my understanding of the plan. The integration with every managed thread hasn't been discussed or considered to my knowledge.

It was discussed at some point, but I forget where and when and with whom.

It was my understanding that there would need to be an autorelease pool for every thread that may run managed code, including finalizer thread, tiering background thread, multi-core JIT background thread, etc.

Correct.

For externally created threads that enter the runtime to run managed code, I'm not sure if an autorelease pool is needed. May depend on how the thread was created, guess it wouldn't hurt to have a pool for that case too.

I'm not sure either, but it won't hurt to have the pool for this case too (at the very least it's theoretically possible to need it).

Should I file a new issue for this, or should it be incorporated into this issue?

@lambdageek
Copy link
Member

I'm not sure either, but it won't hurt to have the pool for this case too (at the very least it's theoretically possible to need it).

I need to trace through the profiler API to see if the event that xamarin-macios subscribes to is raised for external threads that attach to the runtime. I suspect we do not.

I agree that it seems like one could come up with an example where it is necessary.

@AaronRobinsonMSFT
Copy link
Member Author

AaronRobinsonMSFT commented Apr 26, 2021

@rolfbjarne I think creating one for every managed thread is relatively easy to accomplish, but there is the question of when to drain that pool. Is this something we need to handle in the runtime or is that handled somewhere else? I didn't see the case in xamarin-macios where the NSAutoreleasePool is drained other than when the thread is terminated. Want to confirm my understanding here.

@rolfbjarne
Copy link
Member

@rolfbjarne I think creating one for every managed thread is relatively easy to accomplish, but there is the question of when to drain that pool. Is this something we need to handle in the runtime or is that handled somewhere else? I didn't see the case in xamarin-macios where the NSAutoreleasePool is drained other than when the thread is terminated. Want to confirm my understanding here.

The pool should be drained when the thread terminates, it spans the entire lifetime of the thread.

@lambdageek
Copy link
Member

I'm not sure either, but it won't hurt to have the pool for this case too (at the very least it's theoretically possible to need it).

I need to trace through the profiler API to see if the event that xamarin-macios subscribes to is raised for external threads that attach to the runtime. I suspect we do not.

Mono raises the thread_started event from fire_attach_profiler_events

https://github.com/dotnet/runtime/blob/94d41d70d7038d94149d485a0f153eb8012859b3/src/mono/mono/metadata/threads.c#L1093

fire_attach_profiler_events is called when new threads are created, and also from the thread attach machinery when a foreign thread attaches to the runtime.

@AaronRobinsonMSFT
Copy link
Member Author

@lambdageek Thanks for the confirmation. I will update the prototype with that logic.

Co-authored-by: Jan Kotas <jkotas@microsoft.com>
@AaronRobinsonMSFT
Copy link
Member Author

@rolfbjarne @lambdageek @jkoritzinsky @elinor-fung Please take a look at this design and ensure it aligns with your understanding of our Objective-C interop story.

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.

9 participants