Open Manishearth opened 2 months ago
@Manishearth When I was first playing with jvm ffi stuff, it also occurred to me that Box<T>
should be enough for thread safe access, because the GC should take care of cleanup, so we don't need reference counting, as long as we have T: Send + Sync
as you said. I managed to get mutable access with Box<Mutex<T>>
without leaks (which is a pattern I never thought I'd see), but adds some nastiness, as now we can implicitly get deadlocks if we're not careful. If we went down this path it would be nice if there could be some kind of guaranteed lock order for methods accepting multiple mutable references. It might be more reasonable to keep synchronization in the backend language. May be relevant for https://github.com/rust-diplomat/diplomat/issues/225
It might be more reasonable to keep synchronization in the backend language
As in, the backend manages synchronization? How would this look for, say, Java or Kotlin?
Also for the purposes of this discussion it may be worth assuming #225 as a prerequisite
If we had the opaque_mut
attribute then we could automatically add a locking interface to the class, then we could add a random id that's used for global lock order
like
interface Lockable {
final ReadWriteLock lock = new ReentrantReadWriteLock();
final UUID lockId = UUID.randomUUID();
}.
public class MyString implements Lockable {
...
}
Then when we actually want to call them we order the locks with the UUIDs
public class SomeClass {
public ReturnType someMethod(MyString myStr1, MyString myStr2) {
var locks = new Lockable[]{myStr, myStr2}; // Here we could probably combine with a custom accessor to preferentially choose how to lock it (read or write depending on the reference access type, below I just defaulted to write)
Arrays.sort(locks, new Comparator<Lockable>() {
@Override
public int compare(Lockable lockable1, Lockable lockable2 ) {
return lockable1.lockId.compareTo(lockable2.lockId);
}
});
Arrays.stream(locks).forEach(myString -> myString.lock.writeLock().lock());
// Regular conversions and method call
...
Arrays.stream(locks).forEach(myString-> myString.lock.writeLock().unlock()); // this can be added to cleanups if any parameters of a method are `opaque_mut`
return returnVal;
}
}
...
Makes sense. opaque_mut
is not super hard to add, it would just be an additional field on Opaque
and we'd need a validation pass after HIR lowering.
The current set of "primary" Diplomat backends (C++, C, JS, Dart) do not need to care about thread safety: C and C++ are places where the "norm" is that you need to manually document/figure out thread safety, and JS/Dart don't let objects jump to other threads.
However, this isn't ideal for C/++ and this won't work for Java/Kotlin/etc. References to opaque types transferred to other threads will be able to deal with mutable state concurrently.
I'm not sure there is a one-size-fits-all solution here. The general thrust of any solution will be:
Sync
andSend
bydiplomat-macro
&mut
by HIR loweringIt occurred to me to return such types as
Arc<T>
instead ofBox<T>
but I don't actually think that buys us anything that just "enforcing Send and Sync" doesn't.Probably needs #225