On Monday, 22 October 2018 at 10:26:14 UTC, Timon Gehr wrote:
module borked;
void atomicIncrement(int* p)@system{
import core.atomic;
atomicOp!("+=",int,int)(*cast(shared(int)*)p,1);
}
struct Atomic(T){
private T val;
void opUnary(string op : "++")() shared @trusted {
atomicIncrement(cast(T*)&val);
}
}
void main()@safe{
Atomic!int i;
auto a=&[i][0];// was: Atomic!int* a = &i;
import std.concurrency;
spawn((shared(Atomic!int)* a){ ++*a; }, a);
++i.val; // race
}
---
Oh no! The author of the @trusted function (i.e. you) did not
deliver on the promise they made!
Now, before you go and tell me that I am stupid because I wrote
bad code, consider the following:
- It is perfectly @safe to access private members from the same
module.
Yes, so you need to place the @trusted code in a separate module.
The @trusted code will be very rare (Atomic!T, lockfree queue,
mutex, semaphore...). This will be in the standard library,
possibly some other library. You should not be writing your own
implementations of these. You should not be writing @trusted code
without very good reason. You should be using @safe functions all
over your code if at all possible.
- You may not blame the my @safe main function for the problem.
It is @safe, so it cannot be blamed for UB. Any UB is the
result of a bad @trusted function, a compiler bug, or hardware
failure.
No, we blame the fact you have not blocked off non-thread-safe
access to Atomic!T.val. What you have made is this:
https://i.imgur.com/PnKMigl.jpg
The piece of code to blame is the @trusted function - you can't
trust it, since non-thread-safe access to Atomic!T.val has not
been blocked off. As long as anyone can extend its interface,
Atomic!T can't be thread-safe.
I'll quote myself:
For clarity: the interface of a type is any method, function,
delegate or otherwise that may affect its internals. That
means any free function in the same module, and any
non-private members.
I've actually missed some possibilities there - member functions
of other types in the same module must also count as part of the
interface. Because of this wide net, modules that implement
thread-safe types with shared methods should be short and sweet.
- The only @trusted function in this module was written by you.
You said that there is a third implementation somewhere. If
that one actually works, I apologize and ask you to please
paste it again in this subthread.
Here's the correct version:
module atomic;
void atomicIncrement(int* p) @system {
import core.atomic;
atomicOp!("+=",int,int)(*cast(shared(int)*)p,1);
}
struct Atomic(T) {
// Should probably mark this shared for extra safety,
// but it's not strictly necessary
private T val;
void opUnary(string op : "++")() shared @trusted {
atomicIncrement(cast(T*)&val);
}
}
---------
module unborked;
import atomic;
void main() @safe {
auto a = new Atomic!int;
import std.concurrency;
spawn((shared(Atomic!int)* a){ ++*a; }, a);
//++i.val; // Cannot access private member
}
Once more, Joe Average Programmer should not be writing the
@trusted code in Atomic!T.opUnary - he should be using libraries
written by people who have studied the exact issues that make
multithreading hard.
--
Simen
