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

Assembly.Location returns / instead of the actual path intermittently #90657

Closed
jkotas opened this issue Aug 16, 2023 · 5 comments · Fixed by #90753
Closed

Assembly.Location returns / instead of the actual path intermittently #90657

jkotas opened this issue Aug 16, 2023 · 5 comments · Fixed by #90753

Comments

@jkotas
Copy link
Member

jkotas commented Aug 16, 2023

Large internal Microsoft workload observed Assembly.Location sometimes returning / instead of the actual path after upgrading to recent .NET 8 build on Linux.

@ghost ghost added the untriaged New issue has not been triaged by the area owner label Aug 16, 2023
@dotnet-issue-labeler dotnet-issue-labeler bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Aug 16, 2023
@jkotas jkotas added area-AssemblyLoader-coreclr and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Aug 16, 2023
@ghost
Copy link

ghost commented Aug 16, 2023

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

Issue Details

Large internal Microsoft workload observed Assembly.Location sometimes returning / instead of the actual path after upgrading to recent .NET 8 build on Linux.

Author: jkotas
Assignees: -
Labels:

area-AssemblyLoader-coreclr, untriaged, needs-area-label

Milestone: -

@jkotas
Copy link
Member Author

jkotas commented Aug 16, 2023

I suspect that this is a race condition caused by SString internally changing its encoding.

One thread can be changing the encoding from UTF8 to UTF16 while the other thread is operating on the string. The other thread may see encoding set to UTF8, but the actual payload can be converted to UTF16 already. It would explain how the method can return / instead of the actual path /abc/def/assembly.dll.

@AaronRobinsonMSFT @jkoritzinsky You are familiar with how SString works. Would you mind reviewing all places that access PEImage::m_path and check whether there may be a race condition like what I have described? (I have taken a cursory look, but I have not found anything obvious.)

@jkotas jkotas changed the title Assembly.Location returns / intermittently Assembly.Location returns / instead of the actual path intermittently Aug 16, 2023
@mangod9
Copy link
Member

mangod9 commented Aug 16, 2023

the race wouldnt be Linux specific would it?

@jkotas
Copy link
Member Author

jkotas commented Aug 16, 2023

the race wouldnt be Linux specific would it?

Depends. We have many Windows vs. non-Windows ifdefs in the vm. The race condition can be triggered by a code under non-Windows ifdef.

@AaronRobinsonMSFT
Copy link
Member

the race wouldnt be Linux specific would it?

Depends. We have many Windows vs. non-Windows ifdefs in the vm. The race condition can be triggered by a code under non-Windows ifdef.

It is also possible, quite likely actually, this is on all platforms and the Linux workload in question was simply the first to find it.

@agocke agocke added this to AppModel Aug 17, 2023
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Aug 17, 2023
@agocke agocke added this to the 8.0.0 milestone Aug 17, 2023
@ghost ghost removed the untriaged New issue has not been triaged by the area owner label Aug 17, 2023
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Aug 24, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Sep 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants