loading: on non-windows use libdl to load libfdb_c

[abdullin]

kernel32 is only available on Windows. In order to load libraries from non-Windows, we need to refer to the libdl (or dl).

I introduced method LoadPlatformLibrary which selects the proper method based on the current runtime. Also dropped FreeLibrary, since it is no longer used.

[KrzysFR]

The FreeLibrary is not used because it would completely crash everything until #54 is resolved, so it may come back later.

Could you also try to look at the static ctor of FdbNative? This is the place that is forcing you to disable preload, and which is win32 only. Maybe do something like if (_platform_is_windows_) { existing_behavior(); } else { plateform_neutral_behavior(); } ? Keeping the current code that checks the .dll extension inside the win32 specific code.

I'll try to revisit this part at a later time, but at least you could unlock support for mac/linux for now.

[abdullin]

Sure. I'll take a look and update the PR later.

…

On Fri, Apr 27, 2018, 20:25 Christophe Chevalier ***@***.***> wrote: The FreeLibrary is not used because it would completely crash everything until #54 <#54> is resolved, so it may come back later. Could you also try to look at the static ctor of FdbNative? This is the place that is forcing you to disable preload, and which is win32 only. Maybe do something like if (_platform_is_windows_) { existing_behavior(); } else { plateform_neutral_behavior(); } ? Keeping the current code that checks the .dll extension inside the win32 specific code. I'll try to revisit this part at a later time, but at least you could unlock support for mac/linux for now. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#79 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAezzuu-WZSHfnfTx5JdFtrWjxvDtRXFks5tszhNgaJpZM4Tqj4O> .

[abdullin]

Ideally, in order to keep things clean, we could set FDB_C_DLL to be fdb_c instead of fdb_c.dll. This doesn't change Windows behavior, but:

1. Makes PInvoke work out-of-the box if native loading is disabled
2. Makes a cleaner code inside the static FdbNative ctor non-empty path (and we could still preserve existing windows-specific behavior).

This specific change shouldn't affect anything for the Windows runtime. In fact LoadLibrary PInvoke already uses kernel32 name instead of kernel32.dll.

What do you think?

[KrzysFR]

Yes that should help fix this.

[abdullin]

Apparently DllImport("fdb_c") and LoadLibrary("fdb_c") don't work exactly the same.

DllImport - does smart dll loading (using suffixes and prefixes), while dlopen tries to load exactly. And since the default the behaviour of FoundationDB.Client is to do preload the native library via LoadLibrary, my implementation doesn't work out of the box.

Currently I don't see a way to preserve existing Windows behaviour (of pre-loading native library) while keeping the logic simple and doing proper OSX/Linux loading at the same time. Probably need to sleep over it :)

[KrzysFR]

OK, I was able to make it run on macOS with .NET Core 2.1, with your changes and some hacks here and there. I can get fdbshell to run somehow (I'm getting some weird bugs that, but the core fdb API seems to work).

I'll try to get some time this week end to hack on this a little more (I'm using VSCode but it's not liking the projects that are still targeting .NET 4.6.1). Also, I'm still unable to run tests from the code, so this is a little bit restrictive.

[KrzysFR]

Also, I think that on mac and probably linux, the PATH resolution for libfdbc "just works" by defaut. I installed the .pkg, did a dotnet run and it picked up the lib immediately.

I think this will simplify things a lot for those platform, and only win32 will need to support the legacy PATH resolution.

[abdullin]

Ok, I cleaned up the PR (rebasing it along the way). We keep the existing library loading behaviour on windows, but opt out for a simpler flow on non-Win: if library path is explicitly provided, then preload that version, otherwise just let PInvoke work.

[KrzysFR]

Looks great! I think we even may be able to get rid of the #define MonoCS at some point