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

Error out when NativeLib has EventPipe enabled #90811

Merged
merged 3 commits into from
Aug 19, 2023

Conversation

LakshanF
Copy link
Contributor

EventPipe does not work correctly with native AOT library (#89346). We will generate an error when EventSourceSupport property is set for native aot library.

@ghost
Copy link

ghost commented Aug 18, 2023

Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas
See info in area-owners.md if you want to be subscribed.

Issue Details

EventPipe does not work correctly with native AOT library (#89346). We will generate an error when EventSourceSupport property is set for native aot library.

Author: LakshanF
Assignees: LakshanF
Labels:

area-NativeAOT-coreclr

Milestone: -

…e.Publish.targets

Co-authored-by: Andy Gocke <andy@commentout.net>
@@ -50,6 +50,7 @@
Text="RuntimeIdentifier is required for native compilation. Try running dotnet publish with the -r option value specified." />
<Error Condition="'$(GeneratePackageOnBuild)' == 'true'" Text="GeneratePackageOnBuild is not supported for native compilation." />
<Error Condition="'$(OutputType)' != 'Library' and '$(NativeLib)' != '' and '$(CustomNativeMain)' != 'true'" Text="NativeLib requires OutputType=Library." />
<Error Condition="'$(NativeLib)' != '' and '$(EventSourceSupport)' == 'true'" Text="EventSource is not supported when compiling to a native library. Set EventSourceSupport to false." />
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we update the native AOT diagnostics doc you added to call out that it is not supported for native libraries?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure on this one, as in this is too much of a corner case to call out on the official documentation and this error should be sufficient enough.

…e.Publish.targets

Co-authored-by: Jan Kotas <jkotas@microsoft.com>
@LakshanF LakshanF merged commit e811599 into dotnet:main Aug 19, 2023
@LakshanF LakshanF deleted the LibAotEventPipeFix branch August 19, 2023 21:33
@agocke
Copy link
Member

agocke commented Aug 22, 2023

/backport to release/8.0

@github-actions
Copy link
Contributor

Started backporting to release/8.0: https://github.com/dotnet/runtime/actions/runs/5941905358

MichalStrehovsky added a commit to MichalStrehovsky/runtime that referenced this pull request Sep 7, 2023
MichalStrehovsky added a commit that referenced this pull request Sep 14, 2023
Works around #89346.

We blocked event pipe completely in shared libraries in #90811 but:

* Elinor found out the shutdown problem is actually Windows specific, so blocking Linux (our most important target) is unnecessary.
* We can avoid the hang at the cost of corrupting the ongoing event pipe session by just not doing the shutdown. This can be worked around by simply stopping the event pipe session at dotnet-trace side manually.

I validated the shared library scenario no longer hangs at shutdown with this.

#89346 still tracks if we can do better here.
github-actions bot pushed a commit that referenced this pull request Sep 14, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Sep 21, 2023
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.

4 participants