search

stil / CurlThin

Lightweight cURL wrapper for C# with support for curl_multi polling interface through libuv
Other
68 stars 18 forks source link

This library doesn't work on 32 bit Window builds (x86) #17

Open happypsyduck opened 2 years ago

happypsyduck commented 2 years ago

Using the same libcurl build as the library was built on (libcurl 7.70.0-x86), it causes an exception at curl_easy_perform in the example code (AccessViolation). This occurs even if the CurlThin architecture matches that of the libcurl build (x86).

Builds for 64 bit versions work as expected.

SoLaRGit commented 2 years ago

This is an Issue how Platform Invoke definitions are made. Namely Calling Conventions, ie: Curl uses default C calling convention popularly known as CDecl, where .NET uses default Standard Call (StdCall). When these two conventions used in call it always results in un-ballanced stack and Access Violation. To fix it, all P/Invoke functions should have in x86 builds imports definition of calling convention as presented below

private const CallingConvention CallConv = CallingConvention.Cdecl;

[DllImport(LIBCURL, CallingConvention = CallConv, PreserveSig = true, SetLastError = true, CharSet = CharSet.None, EntryPoint = "curl_global_init")]
public static extern CURLcode Init(CURLglobal flags = CURLglobal.DEFAULT);
// and so on ...

I've also added to preserve the function signature, set last error, and type of marshaling string to use (in this case would be None as there are no strings). In other cases it should be Ansi - when char <- note take good care if buffer is not actually just plain byte array - C programmers know to leave out 'unsigned' keyword and use plain char as byte array. And third case Unicode - when WCHAR, wchar_t, or similar wide char types used it also include when using TCHAR* and library is compiled using UNICODE definition. Leaving out charset definition would let .Net guess which string marshaling to use, which could lead to unpredictable behavior.

The same should be also done with the callback function (as the Curl expect in the signature of callback function to be CDecl as well:

[UnmanagedFunctionPointer(CallingConvention.Cdecl, CharSet = CharSet.Ansi, SetLastError = true)]
public delegate UIntPtr DataHandler ...

Yes I've noticed bad use of t_size in callback where the platform callback function signature changed depending on arch. Only two ways I know to solve this issue is using IntPtr and UIntPtr (SysInt, USysInt), like in your case. Or the other case, having two signatures, and wrapping it inside library obfuscating it from end user, and exposing pure managed callback.

Now making above changes fixes issue with x86 builds, and now the same code works on x64, x86, AnyCPU (ofcourse depending which curl.dll version your have - you must match the arch).

Hope this helps fix this library for the others that are using it.

I am guessing you wonder how AMD64 builds are working just fine, well simply AMD64 arch is ignoring calling conventions, and code works as advertised, or just check documentation (since there is only one calling convention under AMD64) same applies to ARM64: "On ARM and x64 processors, __stdcall is accepted and ignored by the compiler; on ARM and x64 architectures, by convention, arguments are passed in registers when possible, and subsequent arguments are passed on the stack." Similar text on all calling conventions check https://docs.microsoft.com/en-us/cpp/cpp/stdcall?view=msvc-170

Now this being said, this library has huge overhead, since it has packed in resource curl.dll which is extracted once Init function is called. This is bad idea on so many levels, if you wanted to perform this task you should have made build.target that would copy necessary arch library from \package\\native\\ directory to \output\ directory, or anything similar. (Also missing ARM support library)

Considering the level of nuget packages also required for this library to work it is becoming insane for any serious use. Too many dependencies to many ways to go wrong. Some very popular project has been canceled due this problem.

So I do not attend to use it, I'll make my own light version with only dependency to curl.dll (statically linked dependency libs) Following are the list under Windows platform at the time of this writing.

Specifications curl 7.83.0 was built and statically linked with

OpenSSL 3.0.2 [64bit/32bit] brotli 1.0.9 [64bit/32bit] libgsasl 1.10.0 [64bit/32bit] libidn2 2.3.2 [64bit/32bit] libssh2 1.10.0 [64bit/32bit] nghttp2 1.47.0 [64bit/32bit] zlib 1.2.12 [64bit/32bit]