digitalmars.D.learn - Synchronisation help
- Anonymouse (41/41) Jan 01 I have a `shared string[int]` AA that I access from two different
- Jonathan M Davis (62/104) Jan 01 In general, I would advise against using synchronized statements. They
- Anonymouse (87/88) Jan 02 Thank you. Yes, `Foo` is a class for the purposes of inheritance
- Christian =?UTF-8?B?S8O2c3RsaW4=?= (6/20) Jan 02 It might be, that this locking scheme is too narrow. E.g. you
- Jonathan M Davis (166/228) Jan 02 For a member function to work on a shared object, that member function m...
- Anonymouse (8/9) Jan 03 That clarifies a lot. If nothing else, I realise now I can't have
- Steven Schveighoffer (8/13) Jan 01 Yeah, and the thing should be a `Mutex` object. A `Mutex` object
- user1234 (7/16) Jan 02 Do not use `shared` AA. Use `__gshared` + sync primitives.
- Anonymouse (6/12) Jan 02 Hmm, I see.
- Jonathan M Davis (26/39) Jan 02 You should almost never use __gshared. It's really only intended to be u...
- user1234 (4/17) Jan 02 The semantics of __gshared in this context is "no-TLS". You meant
I have a `shared string[int]` AA that I access from two different
threads. The function I spawn to start the second thread takes
the AA as an argument.
```d
class Foo
{
shared string[int] bucket;
Tid worker;
}
void workerFn(shared string[int] bucket)
{
while (true)
{
// occasionally reads, occasionally modifies bucket
}
}
void main()
{
auto foo = new Foo;
foo.bucket[0] = string.init;
foo.bucket.remove(0);
foo.worker = spawn(&workerFn, foo.bucket);
while (true)
{
// occasionally reads, occasionally modifies bucket
}
}
```
(`run.dlang.io` shortening seems broken again, but I made a
[gist](https://gist.github.com/zorael/17b042c424cfea5ebb5f1f3120f983f4) of a
more complete example.)
Reading the specs on `synchronized` statements, it seems I need
to provide an `Object` to base synchronisation on when two
*different* places in the code needs synchronising, whereas if
it's in the same place an expressionless `synchronize { }` will
do.
The worker function can't see `Foo foo` inside `main`, so it
can't share synchronisation on that.
What is the common solution here? Do I add a module-level `Object
thing` and move everything accessing the AA into
`synchronized(.thing)` statements? Or maybe add a `shared static`
something to `Foo` and synchronise with `synchronize(Foo.thing)`?
Jan 01
On Monday, January 1, 2024 8:48:16 AM MST Anonymouse via Digitalmars-d-learn
wrote:
I have a `shared string[int]` AA that I access from two different
threads. The function I spawn to start the second thread takes
the AA as an argument.
```d
class Foo
{
shared string[int] bucket;
Tid worker;
}
void workerFn(shared string[int] bucket)
{
while (true)
{
// occasionally reads, occasionally modifies bucket
}
}
void main()
{
auto foo = new Foo;
foo.bucket[0] = string.init;
foo.bucket.remove(0);
foo.worker = spawn(&workerFn, foo.bucket);
while (true)
{
// occasionally reads, occasionally modifies bucket
}
}
```
(`run.dlang.io` shortening seems broken again, but I made a
[gist](https://gist.github.com/zorael/17b042c424cfea5ebb5f1f3120f983f4) of a
more complete example.)
Reading the specs on `synchronized` statements, it seems I need
to provide an `Object` to base synchronisation on when two
*different* places in the code needs synchronising, whereas if
it's in the same place an expressionless `synchronize { }` will
do.
The worker function can't see `Foo foo` inside `main`, so it
can't share synchronisation on that.
What is the common solution here? Do I add a module-level `Object
thing` and move everything accessing the AA into
`synchronized(.thing)` statements? Or maybe add a `shared static`
something to `Foo` and synchronise with `synchronize(Foo.thing)`?
In general, I would advise against using synchronized statements. They
really don't add anything, particularly since in many cases, you need access
to more complex thread-synchronization facilities anyway (e.g. condition
variables). Really, synchronized statements are just a Java-ism that D got
fairly early on that were arguably a mistake. So, I'd typically suggest that
folks just use Mutex from core.sync.mutex directly (though you can certainly
use them if you don't need to do anything more complex).
https://dlang.org/phobos/core_sync_mutex.html
If you're using synchronized statements, you're essentially just using
syntax which does that underneath the hood without providing you the
functionality to use stuff like Condition from core.sync.condition.
https://dlang.org/phobos/core_sync_condition.html
However, regardless of whether you use synchronized or use Mutex directly,
what you need to do is to have an object that functions as a mutex to
protect the shared data, locking it whenever you access it so that only one
thread can access it at a time.
The best place to put that mutex varies depending on what your code is
doing. A shared static variable could make sense, but it's often the case
that you would put the mutex inside the class or struct that contains the
data that's shared across threads. Or if you don't have a type that's
intended to encompass what you're doing with the shared data, then it often
makes sense to create one to hold the shared data so that the code that's
using it doesn't have to deal with the synchronization mechanisms but rather
all of that mess is contained entirely within the class or struct that
you're passing around. But even if you don't want to encapsulate it all
within a struct or class, simply creating one to hold both the shared data
and the mutex makes it so that they'll be together wherever you're passing
them around, making it easy for the code using the AA to access the mutex.
However, because you're not supposed to actually be mutating data while it's
shared (and the compiler largely prevents you from doing so), what you
generally need to do to operate on shared data is to lock the mutex that
protects it, cast away shared so that you can operate on the data, do
whatever it is that you need to do with the now thread-local data, make sure
that no thread-local references to the data exist any longer, and then lock
the mutex again. And to do that cleanly, it's often nice to create a struct
or class with shared member functions which takes care of all of that for
you so that that particular dance is encapsulated rather than having to deal
with any code that has access to that shared data having to deal with the
synchronization correctly.
Given that you already have a class called Foo which contains the AA, I
would say that the most obvious thing to do would be to just pass a shared
Foo across threads rather than pass the AA from inside Foo. Then you can
either put a mutex in Foo that then naturally gets passed along with the AA,
or you just use the class itself as a mutex - e.g. synchronized(this) {}
IIRC - since classes unfortunately have a mutex built into them to make
synchronized member functions work (which is useful when you want to use
synchronized functions but causes unnecessary bloat for most classes). And
if it makes sense to lock the mutex during entire function calls, you can
just make the member function synchronized rather than having synchronized
statements (though it often doesn't make sense to have locks cover that much
code at once).
Personally, I'd probably just make Foo a struct and pass a shared Foo* to
the other thread rather than bother with a class, since it doesn't look like
you're trying to do anything with inheritance, and in that case, you'd give
it an explicit mutex, since structs don't have mutexes built in. But if you
want to use a class and use its built-in mutex with synchronized, that works
too. Either way, it makes more sense to pass a shared object containing both
the AA and the mutex around than to pass the AA around by itself given that
the whole point of the mutex is to protect that AA.
- Jonathan M Davis
Jan 01
On Monday, 1 January 2024 at 19:49:28 UTC, Jonathan M Davis wrote:[...]Thank you. Yes, `Foo` is a class for the purposes of inheritance -- I left that out of the example. So a completely valid solution is to write a struct wrapper around an AA of the type I need, overload the required operators, and then just drop-in replace the current AA? All array operations would then transparently be between lock and unlock statements. ```d struct MutexedAA(AA : V[K], V, K) { import core.sync.mutex : Mutex; shared Mutex mutex; shared AA aa; void setup() nothrow { mutex = new shared Mutex; mutex.lock_nothrow(); if (K.init !in (cast()aa)) { (cast()aa)[K.init] = V.init; (cast()aa).remove(K.init); } mutex.unlock_nothrow(); } auto opIndexAssign(V value, K key) in (mutex, typeof(this).stringof ~ " has null Mutex") { mutex.lock_nothrow(); (cast()aa)[key] = value; mutex.unlock_nothrow(); return value; } auto opIndex(K key) in (mutex, typeof(this).stringof ~ " has null Mutex") { mutex.lock_nothrow(); auto value = (cast()aa)[key]; mutex.unlock_nothrow(); return value; } auto opBinaryRight(string op : "in")(K key) in (mutex, typeof(this).stringof ~ " has null Mutex") { mutex.lock_nothrow(); auto value = key in cast()aa; mutex.unlock_nothrow(); return value; } auto remove(K key) in (mutex, typeof(this).stringof ~ " has null Mutex") { mutex.lock_nothrow(); auto value = (cast()aa).remove(key); mutex.unlock_nothrow(); return value; } auto opEquals()(auto ref typeof(this) other) in (mutex, typeof(this).stringof ~ " has null Mutex") { mutex.lock_nothrow(); auto isEqual = (cast()aa == cast()(other.aa)); mutex.unlock_nothrow(); return isEqual; } auto opEquals()(auto ref AA other) in (mutex, typeof(this).stringof ~ " has null Mutex") { mutex.lock_nothrow(); auto isEqual = (cast()aa == other); mutex.unlock_nothrow(); return isEqual; } } ``` (https://gist.github.com/zorael/433c50f238b21b9bb68d076d8a495045) I tried this and it seems to work. Is it glaringly incorrect somehow, or am I free to roll with this? You mention passing a `shared Foo*`. In the gist I pass the instance of the `MutexedAA!(string[int])` to the worker thread *by value* instead of as something `shared`, since I couldn't get operator overloading to work when `shared`. (Calling `sharedAA.opIndexAssign("hello", 42)` worked, but `sharedAA[42] = "hello"` wouldn't compile.) I guess this can break synchronisation between the two if I replace the `Mutex` in either thread. Are there any other obvious caveats?
Jan 02
Christian =?UTF-8?B?S8O2c3RsaW4=?= <christian.koestlin gmail.com> On Tuesday, 2 January 2024 at 10:41:55 UTC, Anonymouse wrote:On Monday, 1 January 2024 at 19:49:28 UTC, Jonathan M Davis wrote:It might be, that this locking scheme is too narrow. E.g. you might want to have an "atomic" testAndSet on the AA e.g. check if an element is in and iff its not in put it there. Kind regards, Christian[...]Thank you. Yes, `Foo` is a class for the purposes of inheritance -- I left that out of the example. So a completely valid solution is to write a struct wrapper around an AA of the type I need, overload the required operators, and then just drop-in replace the current AA? All array operations would then transparently be between lock and unlock statements. [...] I guess this can break synchronisation between the two if I replace the `Mutex` in either thread. Are there any other obvious caveats?
Jan 02
On Tuesday, January 2, 2024 3:41:55 AM MST Anonymouse via Digitalmars-d-learn wrote:On Monday, 1 January 2024 at 19:49:28 UTC, Jonathan M Davis wrote:For a member function to work on a shared object, that member function must be marked as shared. For most types, of course, it's completely inappropriate to mark a member function as shared, since those types do nothing with thread synchronization, but for types which are specifically designed to work across threads, it's appropriate to mark the member functions as shared and then to have them internally do whatever they need to do to protect access across threads. So, if you want to create a type which wraps an AA to make it thread-safe, it would be appropriate to make all of its member functions shared and then deal with the locking primitives internally. That being said, as Christian pointed out, whether locking at the operation level is the best way to deal with thread synchronization depends on what you're doing. For instance, doing something like if (auto value = key in aa) { // do stuff with *value } would be seriously problematic if you simply wrapped the AA, because you'd then have a pointer to a value in the AA which might not even be in the AA anymore when you try to do something with it (since another thread could have happily mutated the AA via another function call). In addition, it's problematic if in returns a thread-local pointer, since it's referencing shared data. So, it's a bug for it to be returning a thread-local pointer. It either needs to be returning a shared pointer (meaning that the locking primitives need to be at a higher level), or you need to copy the data out rather than return a pointer (as well as ensuring that the data itself is fully thread-local, which could be problematic with reference types). For something like this, locking the mutex for a whole set of operations makes more sense, and if you're doing that, you probably don't want a struct or class which simply wraps the AA. Rather, you'd want to have whatever code was operating on the AA to be handling the locking - which would often be inside of a struct or class that has the AA as a shared member variable. So, all of the code that uses the AA would be encapsulated, but you wouldn't have created a type that's simply wrapping the AA. What you'd typically do would probably be one of two approaches: 1. Create a type which handles all of the threading stuff internally (including spawning the other thread) and which provides an API to the main thread which is thread-local in the sense that the main thread doesn't have to know or care about the threading that's being done internally. 2. Create a type which is passed from one thread to another and then designed to be used across threads in a thread-safe manner where any operation that you can do on that type is marked as shared and designed to be thread-safe, which would typically mean having the operations being high enough level that the caller doesn't have to worry about the synchronization primitives at all, though of course, depending on what you're doing, you might have to expose more. Either way, the idea would be to make it so that that shared object is handling as much of the threading synchronization stuff as possible, and when it doesn't, it provides the means for the code using it to use the thread synchronization mechanisms on the data in as simple and safe a manner as possible. You could of course have a much messier approach where nothing is really wrapped, but that makes it much harder to manage the code, whereas it will usually work much better if you can encapsulate the stuff that has to deal with shared. But you do have to pick an encapsulation level which allows you to actually protect the data, which isn't likely to be the case at the level of the AA itself but rather at a higher level which is using the AA to do stuff. Ultimately, what you do with sharing data across threads in D is essentially what you'd do in a language like C++. It's just that D's shared makes it clear to the type system what's being shared across threads and what's thread-local so that the type system can assume that most stuff is thread-local as well as prevent you from accessing shared data accidentally (whereas in C++, you only know what's thread-local and what isn't by convention, since normally, _everything_ is shared across threads but only a small portion of it is actusally used by multiple threads). So, we're able to represent in the type system which sections of the code are designed to operate on shared data and which aren't, with the downside that we have to carefully cast away shared within sections of code that actually deal with the appropriate locking primitives. And if that casting is done incorrectly, it can result in objects being treated as thread-local when they aren't, but since shared is part of the type system, you only have to look at the code involving shared to find the sections where you may have screwed up and escaped thread-local references to shared data. So, the way that sharing data across threads works in D is essentially the same as with most C-derived languages, but we have some extra stuff in the type system which makes it harder to shoot yourself in the foot but at the cost that it's more involved to be able to operate on shared data.[...]Thank you. Yes, `Foo` is a class for the purposes of inheritance -- I left that out of the example. So a completely valid solution is to write a struct wrapper around an AA of the type I need, overload the required operators, and then just drop-in replace the current AA? All array operations would then transparently be between lock and unlock statements. ... I tried this and it seems to work. Is it glaringly incorrect somehow, or am I free to roll with this?You mention passing a `shared Foo*`. In the gist I pass the instance of the `MutexedAA!(string[int])` to the worker thread *by value* instead of as something `shared`, since I couldn't get operator overloading to work when `shared`. (Calling `sharedAA.opIndexAssign("hello", 42)` worked, but `sharedAA[42] = "hello"` wouldn't compile.) I guess this can break synchronisation between the two if I replace the `Mutex` in either thread. Are there any other obvious caveats?When sharing stuff across threads, it needs to be in a place where both threads can access the exact same data. If you pass an object across by value, then the object itself is not actually shared across threads (even if it's typed as shared), and no value types within the object are actually shared across threads - just whatever pointer or reference types are contained within the object, and that can get particularly problematic when you have pseudo-reference types - i.e. a type which has some stuff which lives on the stack on some stuff which lives on the heap, which for instance is what you get with dynamic arrays in D, since the data itself lives on the heap, but the pointer and length live on the stack. shared pointers and references inside a shared value type would have the data that they point to shared across threads, but the pointer or reference itself would not be (the same with AAs, which are essentially reference types with special semantics for initialization, which is why you typically need to initialize an AA before passing it around). So, when you're passing objects across threads where the object is supposed to be shared across threads (as opposed to when you're trying to pass ownership across), you typically want it to be either a pointer or reference. That way, everything within the object is shared across threads. So, I would suggest that you design a type which contains both the AA and its associated mutex where it deals with all of the locking internally but has shared member functions so that the code using that object doesn't need to deal with the locking primitives. However, as discussed above, it will need to have operations which are high enough level that all of the locking can be done safely internally (which is not necessarily the case with individual operations on an AA). However, since the best way to do that very much depends on what exactly you're doing, I can't really say how you need to go about it, but you do need to make sure that you don't do stuff like escape thread-local pointers to the shared data (like the opBinaryRight function you have does), and you need to make sure that you don't have issues with data being potentially out-of-date if you release the lock before doing something with that data which requires that the data not be out-of-date.```d struct MutexedAA(AA : V[K], V, K) { import core.sync.mutex : Mutex; shared Mutex mutex; shared AA aa; void setup() nothrow { mutex = new shared Mutex; mutex.lock_nothrow(); if (K.init !in (cast()aa)) { (cast()aa)[K.init] = V.init; (cast()aa).remove(K.init); } mutex.unlock_nothrow(); }Note that this is what a constructor would normally be for, and if you absolutely need something like this to run before using the type, then a class would potentially be a better fit, since then you can have a default constructor - and as discussed above, you probably want to be using a type which is shared across threads via a pointer or reference anyway, so it may make more sense to just use a class in this case - though as also discussed above, you probably don't want a type that simply wraps the AA, so what this type should look like probably needs to be rethought anyway.auto opIndexAssign(V value, K key) in (mutex, typeof(this).stringof ~ " has null Mutex") { mutex.lock_nothrow(); (cast()aa)[key] = value; mutex.unlock_nothrow(); return value; }Note that in general, whether assigning keys or values like this works properly depends on whether the data can be accessed across threads. If they're value types, it's fine, but if they're not, then no other parts of the program can have references to the objects being passed in, or you're going to have problems when accessing them from another thread. This goes for passing data across threads in general. Either the objects need to be of types which are designed to be used as shared (and are therefore operated on as shared rather than having shared cast away, though you probably still need to temporarily cast away shared to get them in and out of the AA), or the thread passing the object to another thread cannot retain a reference to that object. So, you have to worry about more than just the AA itself when worrying about thread-safety. You also need to worry about what you're putting in it and what you do when you take it out. As such, if the types of the key or value are not value types, it might make sense to mark them as shared on the function parameters so that it's clear to the caller that the data is being shared across threads. Note however that the situation is slightly more complex with the keys. With the values, you know that they're being shared across threads, whereas with the keys, it depends on the exact function being called. For instance, opIndexAssign would potentially end up storing the key object inside of the AA (meaning that it has to be treated as shared just like the value), whereas opIndex would just be using the key for hashing and comparison, so it wouldn't need to worry about treating the key as shared.auto opIndex(K key) in (mutex, typeof(this).stringof ~ " has null Mutex") { mutex.lock_nothrow(); auto value = (cast()aa)[key]; mutex.unlock_nothrow(); return value; }Again, the data needs to have been unique when being put in the AA, or you'll have problems here when you take it out. In addition, even if it's unique, since this function leaves the data within the AA, if the type being returned is not a value type, then you have to be sure that no other thread can access the data while this thread is operating on it, which means that the locking primitives likely need to be at a higher level.auto opBinaryRight(string op : "in")(K key) in (mutex, typeof(this).stringof ~ " has null Mutex") { mutex.lock_nothrow(); auto value = key in cast()aa; mutex.unlock_nothrow(); return value; }Note that this opBinaryRight is escaping a thread-local pointer to shared data, which violates thread-safety. If you're going to escape a pointer to the shared data, then that pointer needs to be shared (which really means that the locking mechanism needs to be at a higher level so that it protects access to the data while it's beeing used), or the data needs to be deep copied so that you're dealing entirely with thread-local data rather than a thread-local pointer to shared data. So, hopefully, that clarifies some things, but you do need to be very aware of what data is escaping when casting away shared or casting to shared, since you don't want to be operating on thread-local references to shared data without that data being protected by the appropriate thread synchronization primitives. The type system will prevent you from doing much with shared in general without casting away shared, but you're then on your own with any code where you cast away shared, just like you're on your own with verifying that trusted code is actually safe, since the compiler can't do it for you. - Jonathan M Davis
Jan 02
On Tuesday, 2 January 2024 at 18:01:55 UTC, Jonathan M Davis wrote:[...]That clarifies a lot. If nothing else, I realise now I can't have `opBinaryRight(string op : "in")` the way I had hoped. What I have now probably doesn't cover 100% of every use-case, but it should do for my scope. I'm much more confident using this compared to the naked `shared` AA I started with. Thanks everyone for the help.
Jan 03
On Monday, 1 January 2024 at 15:48:16 UTC, Anonymouse wrote:What is the common solution here? Do I add a module-level `Object thing` and move everything accessing the AA into `synchronized(.thing)` statements? Or maybe add a `shared static` something to `Foo` and synchronise with `synchronize(Foo.thing)`?Yeah, and the thing should be a `Mutex` object. A `Mutex` object uses it's underlying mutex primitive as its monitor. This also gives options for usage with methods as well as `synchronized` statements. Just make sure you mark it `__gshared` or `shared` so all threads see it. -Steve
Jan 01
On Monday, 1 January 2024 at 15:48:16 UTC, Anonymouse wrote:I have a `shared string[int]` AA that I access from two different threads. The function I spawn to start the second thread takes the AA as an argument. [...] What is the common solution here? Do I add a module-level `Object thing` and move everything accessing the AA into `synchronized(.thing)` statements? Or maybe add a `shared static` something to `Foo` and synchronise with `synchronize(Foo.thing)`?Do not use `shared` AA. Use `__gshared` + sync primitives. `shared` AA will lead to all sort of bugs: - https://issues.dlang.org/show_bug.cgi?id=20484#c1 - https://issues.dlang.org/show_bug.cgi?id=17088 - https://issues.dlang.org/show_bug.cgi?id=16597 - etc.
Jan 02
On Tuesday, 2 January 2024 at 11:05:33 UTC, user1234 wrote:Do not use `shared` AA. Use `__gshared` + sync primitives. `shared` AA will lead to all sort of bugs: - https://issues.dlang.org/show_bug.cgi?id=20484#c1 - https://issues.dlang.org/show_bug.cgi?id=17088 - https://issues.dlang.org/show_bug.cgi?id=16597 - etc.Hmm, I see. Is `shared` safe to use with AAs *provided* I use sync primitives, or should I favour `__gshared` over `shared`? I was under the impression `__gshared` was only really meant for interfacing with C.
Jan 02
On Tuesday, January 2, 2024 4:39:12 AM MST Anonymouse via Digitalmars-d-learn wrote:On Tuesday, 2 January 2024 at 11:05:33 UTC, user1234 wrote:You should almost never use __gshared. It's really only intended to be used with C global variables. Some folks use __gshared, because they don't like the restrictions that shared places on your code, but the restrictions are there to protect you from accessing shared data when it's not properly protected. In general, what code should be doing is marking variables as shared so that you cannot accidentally access the data, and then in the sections of code where you've properly protected access to the data, you temporarily cast away shared to operate on it. This is obviously a tad annoying, which is why some folks then just use __gshared to shut up the compiler, but it's very much on purpose that things work this way, and if you mark a variable as __gshared, the type system treats it as thread-local, and it's never caught when you try to access the variable without first dealing with the proper synchronization primitives. Unless a type is specifically designed to work as shared (e.g. a class or struct with shared member functions which do all of the appropriate locking and casting internally), it's expected that you're going to have to either cast away shared or use atomics to operate on shared variables of that type. And AAs are designed to be thread-local, so they have no locking mechanisms built in, and you have to deal with the locking primitives yourself as well as casting away shared to then operate on the AA while it's protected. It's a bug when you can do pretty much anything with a shared AA other than pass it around without casting away shared first. - Jonathan M DavisDo not use `shared` AA. Use `__gshared` + sync primitives. `shared` AA will lead to all sort of bugs: - https://issues.dlang.org/show_bug.cgi?id=20484#c1 - https://issues.dlang.org/show_bug.cgi?id=17088 - https://issues.dlang.org/show_bug.cgi?id=16597 - etc.Hmm, I see. Is `shared` safe to use with AAs *provided* I use sync primitives, or should I favour `__gshared` over `shared`? I was under the impression `__gshared` was only really meant for interfacing with C.
Jan 02
On Tuesday, 2 January 2024 at 11:39:12 UTC, Anonymouse wrote:On Tuesday, 2 January 2024 at 11:05:33 UTC, user1234 wrote:The semantics of __gshared in this context is "no-TLS". You meant to access the AA in several threads right ? So you dont want each thread to have its own copy.Do not use `shared` AA. Use `__gshared` + sync primitives. `shared` AA will lead to all sort of bugs: - https://issues.dlang.org/show_bug.cgi?id=20484#c1 - https://issues.dlang.org/show_bug.cgi?id=17088 - https://issues.dlang.org/show_bug.cgi?id=16597 - etc.Hmm, I see. Is `shared` safe to use with AAs *provided* I use sync primitives, or should I favour `__gshared` over `shared`? I was under the impression `__gshared` was only really meant for interfacing with C.
Jan 02