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

Use libdl.so.2 instead of libdl #1499

Merged
merged 2 commits into from
Jul 4, 2023
Merged

Conversation

CasualPokePlayer
Copy link
Contributor

Nearly all (or all if only counting LSB conforming) Linux distros have libdl.so.2, and usually don't have a libdl.so symlink unless dev packages are installed.

Fixes #1492

Nearly all (or all if only counting LSB conforming) Linux distros have libdl.so.2, and usually don't have a libdl.so symlink unless dev packages are installed.

Fixes dotnet#1492
@CasualPokePlayer CasualPokePlayer requested a review from a team as a code owner June 8, 2023 20:56
@CasualPokePlayer
Copy link
Contributor Author

@dotnet-policy-service agree

Copy link
Member

@Perksey Perksey left a comment

Choose a reason for hiding this comment

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

This is a functionally breaking change that will impact compatibility with older operating systems that we support today.

@Perksey
Copy link
Member

Perksey commented Jun 8, 2023

Oh I just saw Beyleys comment on the issue. Happy for them to make the call here but I’m worried about this one.

@Beyley
Copy link
Contributor

Beyley commented Jun 8, 2023

This is a functionally breaking change that will impact compatibility with older operating systems that we support today.

I dont think is breaking on any distro in use today, the LSB was written in jan 1997 and all modern distros have agreed on this, as stated in the other issue, if a distro doesnt comply with LSB, it probably cant run dotnet in the first place, i feel like this is a safe breaking change

@HurricanKai
Copy link
Member

I don't really have much say in this, but I'm also a bit concerned here on several levels.

if a distro doesnt comply with LSB, it probably cant run dotnet in the first place

If it doesn't compy with LSB in this specific scenario, so can only resolve libdl and not libdl.so.2, can it really not run .NET? I'm not so sure.

I also remember that some distro (FreeBSD, I think?) does not have libdl in the first place, but instead resolves a stub. As we load libdl via the .NET loading mechanism, which internally also uses libdl, we absolutely require libdl to be able to resolve itself, which (iirc) this distro cannot do, due to the stub. We worked around this, but similar trickery might be triggered by this.

resolving libdl and libdl.so.2 is not at all the same! It's not that we were or are requiring a file libdl to be present, but the way ld simply works is that when resolving dependencies it will resolve libfoo.so -> libfoo.so.1 -> libfoo.so.1.12, resolving the most specific path.
Really I'm surprised libdl won't resolve to the present libdl.so.2 file, it should. My issue is that libdl should resolve to libdl.so.2 (and a bunch of other variants) but libdl.so.2 requires this exact file to be present (or a subversion like libdl.so.2.123)

After some research it seems this may be rooted in the runtime not adding the .so suffix, which we rely on in this, and other, cases. It's hard to diagnose those as the original issue #1492 does not provide any context as to where the DllNotFoundException is actually thrown or regarding what exact search path. There are various hooks that we could look at to diagnose runtime behavior here.

I myself run arch, but have not encountered this issue, and am unsure how to reproduce exactly.

Also an somewhat unqualified note on the standards mentioned, I cannot find these ever talking about specific file names. There is SONAME, which has only small relations to the filename, and the resolution name, which is also only a subset of the actual filename. We currently use libdl, which is also the listed resolution name, and C compilers should generally use this and follow names the same way we expect ldd to, just C compilers might also look for an SONAME to verify ABI things.

So yeah, I think this is a very fragile system both from our side and from the various distros, it has many issues and just changing it without very good reason and verification is dangerous!

@CasualPokePlayer
Copy link
Contributor Author

CasualPokePlayer commented Jun 9, 2023

*BSDs are not Linux (they're just another Unix branch) and Silk doesn't use libdl for this case, it just directly uses libc.

libdl.so is required for C compilation, yes, but again, unless dev packages are installed (some libc6-dev package) it won't be available often times (this wouldn't affect the final compilation I think since it'd just end up putting the libdl.so.2 in the executable? or probably not at all if glibc >=2.34 is used). (And even then, that's not even a guarantee it will be available in the first place, on a Ubuntu 22.04 machine with libc6-dev packages installed, it only provides a libdl.a, no libdl.so). Some distros might provide it available for programs to dynamically link by default, but certainly not on Ubuntu (the tested OS) and probably not Debian either looking at its package list (and likely extends to all Debian/Ubuntu based OSes), and really it would only maybe be needed when compiling software (I imagine with Arch and AUR you'd end up having compilers/dev packages installed anyways).

(also, not being available being it just doesn't have the symlink to libdl.so.2, no symlink means ld can't resolve it, ld doesn't have any magic here, the magic is in the symlinks)

The DllNotFoundException is thrown on the first invocation of some libdl function used (which then only would occur when the DllImport function could not resolve the dll, or .so in this case).

Also again to be clear, you would need to be using .NET Standard or .NET Framework to reproduce this, along with some distro which does not have a libdl.so (you can usually check this by ldconfig -p | grep libdl.so)

@Beyley
Copy link
Contributor

Beyley commented Jul 2, 2023

(I imagine with Arch and AUR you'd end up having compilers/dev packages installed anyways).

Even on my Arch machine with many AUR packages and compiler etc., there is no libdl.so file, only libdl.so.2, it seems all apps link directly to libdl.so.2, and given the same thing occurs in modern Ubuntu as well, its safe to presume that this is universal change, or is going to be a universal change in the future as distros update glibc, i think it would be safe to merge this, given it fixes a bug on modern machines, and shouldnt cause problems even on very old distros, due to the LSB defining libdl.so.2.

Even though netfx and ns2.0 are basically dead, there is still an amount of people stuck on it due to legacy codebases/libraries, who rely on it when publishing

I feel like this change at most breaks old non-standard distros, and at the least has no effect, this change should definitely be listed on the changelog though.

If you are okay with this being merged @Perksey, id like to get this in, since we have gotten people in the past in the discord still using netfx/mono for some reason or another (like ppc64 linux platforms which have no .NET core runtime and use distro packaged mono)

This is an intentional change by the glibc people btw, libdl is no more, its been consumed by the main libc
https://sourceware.org/pipermail/libc-alpha/2021-August/129718.html

Copy link
Member

@Perksey Perksey left a comment

Choose a reason for hiding this comment

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

Beyleys call

@Beyley
Copy link
Contributor

Beyley commented Jul 4, 2023

Merging for now, can be reverted if unforseen issues arise later on, making a mental note to explain this on the changelog

sidenote: osu-framework has been using libdl.so.2 for years with no issues to my knowledge, so i dont think we will have to revert this https://github.com/ppy/osu-framework/blob/master/osu.Framework/Platform/Linux/Native/Library.cs

@Beyley Beyley merged commit d5f1f29 into dotnet:main Jul 4, 2023
@CasualPokePlayer CasualPokePlayer deleted the patch-1 branch July 6, 2023 15:27
@Perksey
Copy link
Member

Perksey commented Dec 7, 2023

Had a report that this is invoking some weird behaviour on Android, need to take a closer look at this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

libdl.so is not available on many Linux distros, causing crashes on native library load
4 participants