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.

Sign up for GitHub

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

Jump to bottom

Properly Detect & Handle Musl Systems #1657

Merged
merged 16 commits into from
May 20, 2021
Merged

Properly Detect & Handle Musl Systems #1657

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
3037ecc
Properly Musl systems
mattleibow Mar 13, 2021
b0b7596
lazier
mattleibow Mar 13, 2021
4364a3f
lazier-er
mattleibow Mar 13, 2021
08810d1
take 4
mattleibow Mar 13, 2021
8b6e573
take 5
mattleibow Mar 13, 2021
d5ecc56
Merge branch 'main' into dev/musl
mattleibow Mar 19, 2021
624fb2a
Use a better check for glibc
mattleibow Mar 19, 2021
c6c31ea
Make PlatformConfiguration public
mattleibow Mar 20, 2021
ed541a3
Make internal for now
mattleibow Mar 20, 2021
164a7f5
:|
mattleibow Mar 20, 2021
9506288
oops
mattleibow Mar 21, 2021
d068e57
Allow total override of the linux flavor
mattleibow Mar 21, 2021
c9ccdf1
reflection is hard
mattleibow Mar 22, 2021
2df2e5a
Merge branch 'main' into dev/musl
mattleibow Mar 30, 2021
6ccc082
Merge branch 'main' into dev/musl
mattleibow May 18, 2021
c34ba02
this is static and might be in use by the OS
mattleibow May 19, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
81 changes: 40 additions & 41 deletions binding/Binding.Shared/LibraryLoader.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,57 +47,24 @@ static string GetLibraryPath (string libraryName)
var path = typeof (T).Assembly.Location;
if (!string.IsNullOrEmpty (path)) {
path = Path.GetDirectoryName (path);
// 1.1 in platform sub dir
var lib = Path.Combine (path, arch, libWithExt);
if (File.Exists (lib))
return lib;
// 1.2 in root
lib = Path.Combine (path, libWithExt);
if (File.Exists (lib))
return lib;
if (CheckLibraryPath (path, arch, libWithExt, out var localLib))
return localLib;
}

// 2. try current directory
path = Directory.GetCurrentDirectory ();
if (!string.IsNullOrEmpty (path)) {
// 2.1 in platform sub dir
var lib = Path.Combine (path, arch, libWithExt);
if (File.Exists (lib))
return lib;
// 2.2 in root
lib = Path.Combine (lib, libWithExt);
if (File.Exists (lib))
return lib;
}
if (CheckLibraryPath (Directory.GetCurrentDirectory (), arch, libWithExt, out var lib))
return lib;

// 3. try app domain
try {
if (AppDomain.CurrentDomain is AppDomain domain) {
// 3.1 RelativeSearchPath
path = domain.RelativeSearchPath;
if (!string.IsNullOrEmpty (path)) {
// 3.1.1 in platform sub dir
var lib = Path.Combine (path, arch, libWithExt);
if (File.Exists (lib))
return lib;
// 3.1.2 in root
lib = Path.Combine (lib, libWithExt);
if (File.Exists (lib))
return lib;
}
if (CheckLibraryPath (domain.RelativeSearchPath, arch, libWithExt, out lib))
return lib;

// 3.2 BaseDirectory
path = domain.BaseDirectory;
if (!string.IsNullOrEmpty (path)) {
// 3.2.1 in platform sub dir
var lib = Path.Combine (path, arch, libWithExt);
if (File.Exists (lib))
return lib;
// 3.2.2 in root
lib = Path.Combine (lib, libWithExt);
if (File.Exists (lib))
return lib;
}
if (CheckLibraryPath (domain.BaseDirectory, arch, libWithExt, out lib))
return lib;
}
} catch {
// no-op as there may not be any domain or path
Expand All @@ -106,6 +73,38 @@ static string GetLibraryPath (string libraryName)
// 4. use PATH or default loading mechanism
return libWithExt;
}

static bool CheckLibraryPath(string root, string arch, string libWithExt, out string foundPath)
{
if (!string.IsNullOrEmpty (root)) {
// a. in specific platform sub dir
if (PlatformConfiguration.IsMusl) {
var muslLib = Path.Combine (root, "musl-" + arch, libWithExt);
if (File.Exists (muslLib)) {
foundPath = muslLib;
return true;
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If a Musl system, first check the musl-* directories.


// b. in generic platform sub dir
var searchLib = Path.Combine (root, arch, libWithExt);
if (File.Exists (searchLib)) {
foundPath = searchLib;
return true;
}

// c. in root
searchLib = Path.Combine (root, libWithExt);
if (File.Exists (searchLib)) {
foundPath = searchLib;
return true;
}
}

// d. nothing
foundPath = null;
return false;
}
}

public static T GetSymbolDelegate<T> (IntPtr library, string name)
Expand Down
34 changes: 34 additions & 0 deletions binding/Binding.Shared/PlatformConfiguration.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ namespace SkiaSharp
{
internal static class PlatformConfiguration
{
private static readonly Lazy<bool> isMuslLazy = new Lazy<bool>(IsMuslImplementation);

public static bool IsUnix { get; }

public static bool IsWindows { get; }
Expand All @@ -26,6 +28,8 @@ internal static class PlatformConfiguration

public static bool Is64Bit { get; }

public static bool IsMusl => isMuslLazy.Value;

static PlatformConfiguration ()
{
#if WINDOWS_UWP
Expand All @@ -49,6 +53,36 @@ static PlatformConfiguration ()
#endif

Is64Bit = IntPtr.Size == 8;

isMuslLazy = new Lazy<bool>(IsMuslImplementation);
}

private static bool IsMuslImplementation()
{
try
{
var cpu = RuntimeInformation.ProcessArchitecture;
switch (cpu)
{
case Architecture.X86:
return AccessCheck("/lib/libc.musl-x86.so.1", 0) == 0;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Checking for /lib/libc.musl-* seems to be a decent way to detect Musl systems. If all else fails, then we fallback to the existing logic.

Copy link
Contributor

Choose a reason for hiding this comment

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

The existence of that file does not indicate the libc variant that's used by the current process.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

😢 Is there any way to check for Musl? Is there some specific thing that exists in Musl or glibc that we can use to detect? Maybe there is some thing we call that returns a string that says musl or not?

Copy link
Contributor

Choose a reason for hiding this comment

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

What if I told you that technically one system can have both?

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess you could try to DllImport gnu_get_libc_version from libc and see if it throws
EntryPointNotFoundException. If it doesn't, you are definitely running with glibc.

Copy link
Contributor

@kekekeks kekekeks Mar 20, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

BTW, since that check happens at runtime, I'd suggest to expose an API for selecting the proper binary and some SkiaSharp-specific environment variable (e. g. SKIASHARP_LIBC_FLAVOR). So detection logic would look like this:

  1. use API-provided-override if configured
  2. use the value from environment variable if exists
  3. attempt to auto-detect

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. Let me add that.

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 only added 1. as I don't want a library to be reaching out into the environment. This can of course be added by a consuming library or by the final app. However, the API that I am making public is the existing PlatformConfiguration class. This was previously used just to detect the OS, arch or pointer size, however, I added both a IsMusl and IsMuslOverride which can be used to control if the process is glibc or musl:

// use default fallback
PlatformConfiguration.IsMuslOverride = null;
// is Musl
PlatformConfiguration.IsMuslOverride = true;
// is NOT Musl
PlatformConfiguration.IsMuslOverride = false;

If there is ever another libc, we can just add a new IsXlibc and a IsXlibcOverride. Then it still up to the consumer to set the correct values. The recommendation is to only set when you are sure it is one or the other. Although not very high chance we will get another libc, setting it to false when unsure will make the detection checks stop early.

If SkiaSharp is detecting Musl incorrectly, then it should be seen as a bug and hopefully will get reported and fixed so the override should remain mostly unused.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, I am making the API internal for now and can be accessed via reflection. I have tests to make sure it stays.

The reason for this is that this lookup system only exists on mono and we are really starting to move away from that. I added the API for the existing apps that need to control the lookup, but with .NET Core and .NET 5+, all the lookup is done via the runtime features.

And if there is some pluggable system that needs multiple versions of libSkiaSharp in memory, then this is still not an issue because since .NET Core 3.0 there is the NativeLibrary which is the preferred and more correct way to override the loading mechanism: https://docs.microsoft.com/dotnet/api/system.runtime.interopservices.nativelibrary

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, it should actually be using the AssemblyLoadContext and works with the results of a Publish for a project. I created a sample with SkiaSharp based on the docs: https://github.com/mattleibow/SkiaSharpPluggable

case Architecture.X64:
return AccessCheck("/lib/libc.musl-x86_64.so.1", 0) == 0;
case Architecture.Arm:
return AccessCheck("/lib/libc.musl-armv7.so.1", 0) == 0;
case Architecture.Arm64:
return AccessCheck("/lib/libc.musl-aarch64.so.1", 0) == 0;
default:
return false;
}
}
catch
{
return false;
}
}

[DllImport("libc.so", EntryPoint = "access")]
private static extern unsafe int AccessCheck([MarshalAs (UnmanagedType.LPStr)] string path, int mode);
}
}
8 changes: 4 additions & 4 deletions binding/SkiaSharp/nuget/build/net462/SkiaSharp.targets
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,16 +45,16 @@

<!-- Linux: Musl -->
<_NativeSkiaSharpFile Include="$(MSBuildThisFileDirectory)..\..\runtimes\linux-musl-x86\native\libSkiaSharp*.so">
<Dir>x86\</Dir>
<Dir>musl-x86\</Dir>
mattleibow marked this conversation as resolved.
Show resolved Hide resolved
</_NativeSkiaSharpFile>
<_NativeSkiaSharpFile Include="$(MSBuildThisFileDirectory)..\..\runtimes\linux-musl-x64\native\libSkiaSharp*.so">
<Dir>x64\</Dir>
<Dir>musl-x64\</Dir>
</_NativeSkiaSharpFile>
<_NativeSkiaSharpFile Include="$(MSBuildThisFileDirectory)..\..\runtimes\linux-musl-arm\native\libSkiaSharp*.so">
<Dir>arm\</Dir>
<Dir>musl-arm\</Dir>
</_NativeSkiaSharpFile>
<_NativeSkiaSharpFile Include="$(MSBuildThisFileDirectory)..\..\runtimes\linux-musl-arm64\native\libSkiaSharp*.so">
<Dir>arm64\</Dir>
<Dir>musl-arm64\</Dir>
</_NativeSkiaSharpFile>

<!-- macOS -->
Expand Down