ericsink / SQLitePCL.raw

A Portable Class Library (PCL) for low-level (raw) access to SQLite
Apache License 2.0
512 stars 106 forks source link

Add `[UnmanagedCallersOnly]` to `scalar_function_hook_bridge_impl` for WebAssembly support #423

Closed jeromelaban closed 2 years ago

jeromelaban commented 3 years ago

When running on WebAssembly, callbacks needs to be explicitly known at compile time so that code can be generated to replace dynamic stubs that the runtimes with JIT can add.

The .NET runtime adds support for this with .NET 5 by means of UnmanagedCallersOnly (see this for an example).

This scalar_function_hook_bridge_impl callback is used by sqlite3_create_function, at least.

For now, SQLitePCL.SQLite3Provider_sqlite3 is built with netstandard2.0, which does not provide UnmanagedCallersOnlyAttribute as it is available starting from net5.

For information, sqlite3_create_function is used by EF Core here.

ericsink commented 3 years ago

At first glance, we need UnmanagedCallersOnly in exactly the same places that we use the MonoPInvokeCallback attribute.

jeromelaban commented 2 years ago

For Uno's support as well as .NET own tooling, MonoPInvokeCallback is among the lookups done and the SQLitePCLRaw.provider.sqlite3 marks callbacks with it.

The trick here is to ensure that the IL Linker (trimming) does not remove the internal MonoPInvokeCallback defined in SQLitePCLRaw.core. Here's an example of linker file.

The fix there may still be to add UnmanagedCallersOnly to the methods, but could also be to make SQLitePCLRaw.core linker aware.

ericsink commented 2 years ago

@jeromelaban I just pushed up an attempt to integrate a linker file like you described. I was hoping this could be an interim step, simple and low-risk, before I tackle the somewhat larger issue of [UnmanagedCallersOnly] attributes.

Would you mind taking a look at 03c1bdd ? Does it look like I did this right?

Thanks!

ericsink commented 2 years ago

Out-loud conjecture: I shouldn't need both MonoPInvokeCallback and UnmanagedCallersOnly, right? I assume if I put UnmanagedCallersOnly in all the right places, I can consider MonoPInvokeCallback to be the legacy approach and remove it.

jeromelaban commented 2 years ago

Removing it would break the support for xamarin based targets, it'll still be supported for a while. I think you should keep it until Xamarin is out of support and fully replaced by .NET 6/7/8.

ericsink commented 2 years ago

keep it until Xamarin is out of support

Grumble.

So yeah, if the whole switched from Xamarin to net6 all on the same day, then UnmanagedCallersOnly is the replacement for MonoPInvokeCallback.

The question then becomes how long to support old Xamarin. I've really been looking forward to getting desktop msbuild out of my build process. And because Apple/Google usually require everything to be on their latest tools, developers on iOS/Android are accustomed to constant upgrades, so I wasn't expecting to support old Xamarin for very long.

Still thinking about this though. Thanks.

ericsink commented 2 years ago

@bricelam IIRC, you might be working on some pull requests for me involved Wasm support, although I don't know the timing of that effort. I'm tagging you here to let you know that there may be some overlap in what I'm currently doing. For example, as indicated in this issue, I need to support use of the UnmanagedCallersOnly attribute as a substitute for the old MonoPInvokeCallback attribute. But I'm pretty sure I need to do this for net6 xamarin support (which is the thing I'm focusing on), not just for Wasm. So anyway, I thought I should give you a heads-up.

ericsink commented 2 years ago

I think everything here in this issue got dealt with during 2.1.0, which is now released.

jeromelaban commented 2 years ago

Thanks for the update!