Closed elpiel closed 1 month ago
I wasn't able to run this, but random thoughts:
CString
s you allocated might become invalid / dropped even before you call that hook (please double check, I can never remember drop rules), so you might pass invalid pointers in the first placestorage_set
very likely has issues too. Both key
and value
will get dropped at the end of that method, so whatever pointer you passed will be invalid. Ah, as to why Android crashes but Windows not, practically that often doesn't mean anything, as your code might have UB and the only reason it doesn't crash on one platform or another is "luck" (like, some memory living a bit longer on some targets).
* I generally wouldn't pass Storage or any opaque type as a value, do that by reference instead. Service pattern types are not really mean to be allocated / dropped frequently.
Ok, this is good to know. It was working at some point but I guess passing it by value is causing one of the issues.
* Not sure what debug_callback does, but after that returns the `CString`s you allocated might become invalid / dropped even before you call that hook (please double check, I can never remember drop rules), so you might pass invalid pointers in the first place
debug_callback
is just logging debug information for debugging. We were trying to catch where the crash happens.
private void DebugLogCall(string debug_log)
{
Debug.Log("------ "+debug_log);
}
it's also good to add some sort of logging and I'm looking for ways to implement tracing
Subscriber.
Any good tips as to how to check if it's being dropped? Also, apart from moving the CString in separate variable what else can be done?
* Your `storage_set` very likely has issues too. Both `key` and `value` will get dropped at the end of that method, so whatever pointer you passed will be invalid.
Since storage_set is calling
self.set_callback.call(key, value)
(the set callback) wouldn't this mean that C# should first get the pointer to the string and be able to work with it? I did fear that this might be one of the bigger issues, the question is: Since in C# interoptopus creates astring
argument, should we just Copy the value in C# and work with it from there on to avoid invalid pointers?
As for working with Strings and passing them between C# and Rust, what solutions can be used to do this? There's a tokio executor running in Rust with a channel that handles events in the core and we need to handle these events in C# (hence, there's a callback that gets called every time the events thread receives a new event on the channel).
Also regarding the Storage plaque type: Would wrapping the internals in arc be sufficient? We do set the storage to a static which can then be called from everywhere in the application so we need it by value somehow. Or maybe a static lifetime will be enough?
Not sure how she sharp handles these callbacks as well especially in multi-threaded applications.
Ok so wrapping the Storage in an inner Arc<Inner>
and passing it as reference, copying the arc seems to do the job.
As for the debug logging, I separated the creation of the CString from the callback call and putting them in a scope:
{
let log = CString::.....;
debug_callback.call(log.as_c_str())
}
We would continue testing this setup.
As for the callbacks, I'm wondering if it would be an issue the clone them and share across threads or I need to put them in an Arc again?
As for the callbacks, I'm wondering if it would be an issue the clone them and share across threads or I need to put them in an Arc again?
This is hard to answer without looking at the rest of the code and your expectations w.r.t. lifetimes of these callbacks on the C# and their parameters. Maybe I'm going on a tangent here, but as a higher level design recommendation I'd not think about your APIs from a C# perspective, but from a C perspective. With that I mean you shouldn't try to design an API as something you want to use from C#, but as an API you're happy with from C. In fact, when exporting Rust APIs through Interoptopus I still mostly look at the C header files to get an overall feeling for what I'm doing.
If you do that, your API will become more tedious to use, but ultimately better. You might have to jump through some extra hoops dealing with, say, strings, allocations, or "context", for all which you'll have to write extra helper functions. However, that's often the only way to design a C-level API that's sane ("reasonable" in the literal sense) anyways.
Moving this to discussions, this doesn't seem to be a bug.
As discussed in the other issues, I've encountered an issue with the build library for android (.so). When we run most of these methods that call callbacks from C# the application crashes on Android. Windows works for some reason and I've linked the
.cargo/config.toml
file to show how we build the 2 libraries.The .cs files are usually copied to the Unity project.
Repo: https://github.com/elpiel/interoptopus-csharp
Here's also the results of these calls: initialize_native tests - Archive 4.csv
You will notice that apart from
InitializeNativeWithDebugCall
all the rest of the functions actually crash the application under android.InitializeWithStorageWithoutSetGet
&InitializeWithStorageAsciiWithoutSetGet
only have an argument for the Storage impl without using it and they still crash (I've added 2 different impls, one with raw pointers and another with AsciiPointer)It might be due to my mistake in the API, tbh this is the first time I'm developing FFI