shared - i need it to be useful

[Simen Kjærås]

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