[FEATURE] Native library name versioning

[kekekeks]

Consider the following scenario:

• an app with plugin architecture like Visual Studio
• extension Augmented API for Rectangles, Points, Sizes #1 (strong named) uses SkiaSharp 1.60.0
• extension SKBitmap - Surface low-level Lock/Unlock/GetPixels APIs #2 (strong named) uses SkiaSharp 1.68.0
• both extensions get successfully loaded and have different versions of SkiaSharp in the same process
• both versions of SkiaSharp are P/Invoking with the same library name, but only one native dll will be loaded
• one of the versions of SkiaSharp will blow up spectacularly because of C API incompatibility, potentially with access violation

Solution: use library name suffix, e. g.:
libSkiaSharp.1.68.0.dll
libSkiaSharp.1.68.0.dylib
libSkiaSharp.1.68.0.so

[mattleibow]

I'll see what we can do.

This is a real problem, but we might break a lot of people if we just change the names. The pinvoke doesn't really matter, so that is easy. But, this is more the user scripts and things to copy files might stop working.

But, we do need to do this to support cases where side-by-side loading is required.

We might have to wait for the v2 where we can be a little more break-y with file names. I'll move it into that project now so we can make sure we address this.

[mattleibow]

Doing a few tests, seems that this [DllImport] attribute is a tad dumb. If I use the Native.1.0.0 pattern, then it does not add the .dll to it and can't load. So it looks like I will have to do something like Native_68_3 and Native_80_0.

Looks really ugly, but I suppose good looks are not what we are going for here.

[mattleibow]

I think I may have found a nice way to actually solve 2 big issues... First, this issue. Supporting the loading of two versions of libSkiaSharp.* Second, #713. Supporting the actually, intelligent loading of 32/64 bit native binaries.

This is the hacked code for a library... I would just use this on net45 and maybe netstandard2.0. This is unnecessary for packaged apps (Android, iOS, UWP) and actually unsupported on AOT platforms (iOS, WASM). The nice thing is, the generator can do all the work and we can toggle with #if USE_DELEGATE_STYLE or something.

using System;
using System.IO;
using System.Reflection;
using System.Runtime.InteropServices;

namespace Library
{
	public class Class1
	{
		public static int Act() => (test_func ??= (Load<test_delegate>())).Invoke();

		static IntPtr libraryHandle;

		private static T Load<T>()
		{
			// TODO: proper loading logic here
			string assemblyPath = Assembly.GetAssembly(typeof(Class1)).Location;
			if (!string.IsNullOrEmpty(assemblyPath))
				assemblyPath = Path.GetDirectoryName(assemblyPath);
			else
				assemblyPath = Directory.GetCurrentDirectory();

			// TODO: A default fallback of loading "Native" as in the [DllImport] reverts back to the default logic
			libraryHandle = UnsafeNativeMethods.LoadLibrary(Path.Combine(assemblyPath, "Native.dll"));
			var ptr = UnsafeNativeMethods.Win32GetProcAddress(libraryHandle, "test");
			return Marshal.GetDelegateForFunctionPointer<T>(ptr);
		}

		private static test_delegate test_func;

		[DllImport("Native", CallingConvention = CallingConvention.Cdecl)]
		private static extern int test();
	}

	static class UnsafeNativeMethods
	{
		[DllImport("Kernel32.dll", SetLastError = true)]
		public static extern IntPtr LoadLibrary(string lpFileName);

		[DllImport("Kernel32.dll", EntryPoint = "GetProcAddress", CharSet = CharSet.Ansi, ExactSpelling = true, SetLastError = true)]
		public static extern IntPtr Win32GetProcAddress(IntPtr hModule, string lpProcName);
	}

	[UnmanagedFunctionPointer(CallingConvention.Cdecl)]
	delegate int test_delegate();
}

[kekekeks]

Don't attempt loading nuget-shipped libraries with LoadLibrary/dlsym. That will break loading those from $HOME/.nuget directory since coreclr uses some complex logic to resolve paths.

[mattleibow]

Ah, yeah, thanks. I am just doing this for .NET Framework now, so we should be OK. At least this will fix the issues with the 32/64 bit issues on desktop. Things like VS will need to be build as a netfx/net45+ library.

With this CoreCLR work, not sure if this change is actually helpful... But, at least we fixed one issue :|

[ziriax]

Is this useful?

https://github.com/olegtarasov/NativeLibraryManager/blob/master/Readme.md

[mattleibow]

I think we are OK with .NET Core App as that uses the runtimes folder and we never have to worry about anything. I think this issue only really exists with netfx. Unless I am mistaken.

If you are using a .NET Standard library in netfx, you will have the same issue because this is all the runtime... But, since we have a separate netfx library that you should be using anyway, this should never come up. Unless you copy the netstandard dll and use it in a netfx app. But if you shouldn't do that...

[mattleibow]

I think we are OK with .NET Core App as that uses the runtimes folder and we never have to worry about anything. I think this issue only really exists with netfx. Unless I am mistaken.

If you are using a .NET Standard library in netfx, you will have the same issue because this is all the runtime... But, since we have a separate netfx library that you should be using anyway, this should never come up. Unless you copy the netstandard dll and use it in a netfx app. But if you shouldn't do that...

[mattleibow]

Confirmed this works in a VS extensions. So that is nice, and if nothing else we now have fixed 2 issues - the bitness and the VS extension.

[mattleibow]

So I have been playing around and trying a few things. There are a few issues relating to loading of native libraries, so as I was testing things out I think I got to a decent idea. As I type this out, I will think and see what sense I make...

First things first, the issues currently:

1. There is not a good way to pick the correct architecture (x86/x64) when [DllImport] directly on .NET Fx
2. There is not a good way to load multiple versions of native libraries with [DllImport]

Now, the world in which we live:

1. .NET Fx exists and is not that smart, but is powerful
   a) When using [DllImport], Fx will follow a simple lookup path
   b) There is no way to "intercept" a load
   c) Only one library is loaded with a matching name (no multiple versions)
2. .NET Core is smarter, but a little too smart sometimes
   a) When using [DllImport], Core will only look up in the "known"/"special" locations
   b) There is a way to intercept a load
   c) Only one library is loaded with a matching name (no multiple versions)
3. App Packages are not extendable
   a) Apps can only ever look inside the package and can only ever have a single version of a managed dll

And, now the features we need:

1. .NET Fx
   a) Needs to support loading based on arch (x86/x64)
   b) Needs to support loading of different versions
2. .NET Core/.NET Standard
   a) Needs to support loading based on arch AND/OR version (think extensions)
   b) Needs to support loading via magic when used in a .NET Core app - this is like an app package/bundle
3. App Packages
   a) Needs to support AOT and other things that affect loading
   b) Cannot support extensions, so no need for dynamic loading

Finally, we have a few techniques:

1. Loading via LoadLibrary/dlopen and using delegates
   a) This supports any native library arch/version
   b) This breaks the magical lookup of .NET Core
2. Loading via [DllImport] with basic names (libSkiaSharp)
   a) This supports all kinds of magic and is easy
   b) Does not support different versions and no way to control per arch
3. Loading via [DllImport] with versioned names (libSkiaSharp_80_0)
   a) This supports the magic
   b) This supports the multiple version
   c) This does not support different arch

... todo ...

[mattleibow]

So far, a decent looking solution seems to be a combination.

1. .NET Fx library/app
   a) This is dumb and just use delegates to load versioned libraries via LoadLibrary/dlopen
   b) Using delegates allows for arbitrary loading of libraries based on arch/version/path
   c) LoadLibrary/dlopen allows for multiple versions to be used concurrently
2. .NET Standard library in .NET Fx app
   a) This is basically just the same old .NET Fx
   b) Needs to do all the work via delegates
3. .NET Standard library in .NET Core app
   a) This is treated like an app package because the package file will only support a single version of the managed dll
   b) "Extensions" need to manage their own libraries and can use NativeLibrary to override the [DllImport]
4. App Packages
   a) This is the simplest in that it just works as they were
   b) Use the simple [DllImport("libSkiaSharp")]
   c) The build process and runtime will make sure things are correct

Even though is may be nice to just have a .NET Standard library for both .NET Core and .NET Fx, this is not really possible as it doesn't support the delegates AND the magic. This means that we should combine 2. with 1. and 3. with 4. This leaves us with 3 modes:

1. .NET Fx using the net45+ TFM and that uses delegates to load the simple "libSkiaSharp" because we have all the control in the world
2. .NET Standard that works with .NET Core and uses simple [DllImport("libSkiaSharp")] so we can use the magic, extensions override the loading with NativeLibrary.SetDllImportResolver()
3. App packages use simple [DllImport("libSkiaSharp")] to allow the OS/build to do the work

Testing:

1. .NET Fx: I have tested that this works fine and can support multiple versions of SkiaSharp in the VS IDE (.NET Fx)
2. .NET Core: I will make a test app to confirm
3. App Packages: has always been working and no changes needed

[mattleibow]

I can confirm that things seem to be working with, net45+, netcoreapp3.1+ and normal apps.

I create a sample repo: https://github.com/mattleibow/SkiaSharpExtensionsPlayground

[kekekeks]

a) Apps can only ever look inside the package and can only ever have a single version of a managed dll

Note that there are things like ILRepack with internal flag and other IL rewriting options that might be used by packaging scripts

[kekekeks]

Also note that leaving plain libSkiaSharp in DllImport doesn't really help with ABI versioning. We've had "bug" reports from people who were using a wrong package / prebuilt binary for libSkiaSharp.so

[mattleibow]

@kekekeks Thanks for the continued feedback.

Note that there are things like ILRepack with internal flag and other IL rewriting options that might be used by packaging scripts

I was only referring to app store packages/bundles, which has less need for things.

With regards to the IL merging, I don't think this will really be a supported way to distribute libraries. I am investigating a statically linking option for mobile apps that support it (iOS/macOS) and this will result in duplicate symbols. So far my testing has not shown any dramatic size changes, so that is why we are still using dynamic. (or it may be my compiler options for iOS...) The best option would be to have a hybrid solution with dynamic for Debug/Hot Reload and a static for Release/AOT.

In this case, for the App Package format, I don't think we want to support multiple versions of SkiaSharp. If you really need to IL merge two versions of SkiaSharp into an iOS app, then maybe that is a separate case that we can look at when it comes up. If the worst comes to the worst, then you can always Mono.Cecil the attributes and change it to different versions. A bit of work, but can fit into a pretty small MSBuild task.

Also note that leaving plain libSkiaSharp in DllImport doesn't really help with ABI versioning.

I have been looking at versioning the native libraries for the "desktop" platforms (Windows, macOS, Linux) to do this. I have been avoiding this because it makes life a tiny bit harder to maintain from SkiaSharp's perspective, and any additional third party scripting. But it looks to be the best way since the version number is in the name.

We've had "bug" reports from people who were using a wrong package / prebuilt binary for libSkiaSharp.so

I assume you re talking about people pulling down a JoesCodes.SkiaSharp.Linux v60 and using it with SkiaSharp v80?

I suppose adding a version number to the bits will make it more obvious (libSkiaSharp_80 not found vs a crash)

But, another alternative for the last 2 is to add a new API to the native library that provides the version information of the native library.

[Gillibald]

I prefer having an API to query the current version of the native library. Many libraries already have that out of the box. In theory the newer binding could still use an older version of the library if ABI is unchanged.

HarfBuzz for example has such a promise. Most Linux distributions already come with HarfBuzz or offer a way to install the library.

[mattleibow]

@Gillibald I was talking to the folks on the team here, and that is what they also said. Not only does it allow for direct library queries, but also for shared versions across managed libraries - just as you said.

In fact, looking at the releases of SkiaSharp, many of them use the exact same git sha between releases (ie: 1.68.2, 1.68.2.1, 1.68.3) so we could potentially support this far better than just a filename version. And, we can also make use of the fact that the API is being versioned, not the library. So, for example, if the C++ library has a bug and we fix that but do not touch the C API, then we do not have to bump the second version number.

There are probably going to be two numbers used to make up a version: the skia milestone and the C API iteration (restarting with each milestone). For example, the new v2.80.0 will be "80.0". If we do a managed-only update, then the native version is unchanged. If we do a new binding and add a new C API, then the native version becomes "80.1". If we then do another bugfix somewhere in the native code, but do not touch the C API, then we leave the version number as the same.

With regards to backwards compat, I'll make sure that the C API never breaks for a major milestone and is always backwards compatible in that milestone. So, if the native library is "80.2", then this will work with the managed libraries that are compatible with "80.0" to "80.2". But not with a managed library built against "80.3" or later.

Changes	NuGet/Managed	Native
Initial release	v2.80.0	80.0
C# change	v2.80.1	80.0
C API added	v2.80.2	80.2
C++ bug fixed	v2.80.3	80.2
Packaging fix	v2.80.3.1	80.2
C# API added	v2.80.4	80.2
C API added	v2.80.5	80.5
Update skia	v2.84.0	84.0

We do skip iteration values, but that is not so important.