Skip to content

Make Dictionary Send #329

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
RicardRC opened this issue Jul 2, 2023 · 3 comments
Closed

Make Dictionary Send #329

RicardRC opened this issue Jul 2, 2023 · 3 comments
Labels
c: ffi Low-level components and interaction with GDExtension API feature Adds functionality to the library

Comments

@RicardRC
Copy link

RicardRC commented Jul 2, 2023

I don't even know if it possible, but it would be awesome.

Sync would be a blast, too.
I need it to be able to use crossbeam with gdext.

@lilizoey lilizoey added feature Adds functionality to the library c: ffi Low-level components and interaction with GDExtension API labels Jul 2, 2023
@Bromeon
Copy link
Member

Bromeon commented Jul 3, 2023

I don't think it's possible. Reading Godot's thread-safety guidelines:

GDScript arrays, dictionaries

In GDScript, reading and writing elements from multiple threads is OK, but anything that changes the container size (resizing, adding or removing elements) requires locking a mutex.

Since the Dictionary is reference-counted, it would be possible to mutate state from an other thread, which is unsound without mutex. We also don't have access to the reference count itself.

See also #18 and #212.

@lilizoey
Copy link
Member

lilizoey commented Jul 3, 2023

This will be quite an involved addition. It's easy to make a newtype for the dictionary that ensures exclusive access to the dictionary for the right methods. However since a dictionary can store arbitrary values, that means this dictionary could be used to smuggle a non-send value between threads, or to smuggle a reference to a non-sync value between threads.

It would be nice to have such a newtype wrapper, but it'll require a bit of care to make it safe. Perhaps even forbidding the use of Gd<T> values in the dictionary.

How we enforce that is a big harder. We might need some recursive constructor that check every key/value for a non-send/sync value and reject those keys/values. This should probably also come with an unsafe constructor that doesn't check for this.

And of course we'd need to deep duplicate any dictionary used in this newtype.

@Bromeon
Copy link
Member

Bromeon commented Jul 7, 2023

Closing as there is currently no way to safely implement this. For more general discussion about threading design, we can use #18.

If you need this in an unsafe way, you can write a wrapper UnsafeDict(Dictionary) which implements Send -- but then it's your responsibility to keep up the guarantees.

@Bromeon Bromeon closed this as not planned Won't fix, can't repro, duplicate, stale Jul 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: ffi Low-level components and interaction with GDExtension API feature Adds functionality to the library
Projects
None yet
Development

No branches or pull requests

3 participants