On Thursday, 18 October 2018 at 17:23:36 UTC, Stanislav Blinov wrote:
> On Thursday, 18 October 2018 at 17:10:03 UTC, aliak wrote:
>
>> Out of curiosity, when it comes to primitives, what could you do under MP in void "atomicInc(shared int*)" that would be problematic?
>>
>> void atomicInc(shared int*) {
>>   // i.e. what goes here?
>> }
>
> 1. Anything if int* implicitly converts to shared int* (per MP), because then that function is indeed unsafe.

Right, but the argument is a shared int*, so from what I've understood... you can't do anything with it since it has no shared members. i.e. you can't read or write to it. No?

> 2. Only actual platform-specific implementation bugs otherwise, and these are beyond what `shared` can provide.

On Thursday, 18 October 2018 at 18:05:51 UTC, aliak wrote:

> Right, but the argument is a shared int*, so from what I've understood... you can't do anything with it since it has no shared members. i.e. you can't read or write to it. No?

Obviously the implementation would cast `shared` away, just like it would if it were Atomic!int. But for some reason, Manu thinks that latter is OK doing that, but former is voodoo. Go figure.

On Thu, Oct 18, 2018 at 6:40 AM Steven Schveighoffer via Digitalmars-d <digitalmars-d@puremagic.com> wrote:
>
> On 10/17/18 10:26 PM, Manu wrote:
> > On Wed, Oct 17, 2018 at 6:50 PM Steven Schveighoffer via Digitalmars-d
> >>
> >> The implicit cast means that you have to look at more than just your method. You have to look at the entire module, and figure out all the interactions, to see if the thread safe method actually is thread safe. That's programming by convention, and fully trusting the programmer.
> >
> > I don't understand... how can the outer context affect the threadsafety of a properly encapsulated thing?
>
> [snip]
>
> > You need to take it for an intellectual spin. Show me how it's corrupt rather than just presenting discomfort with the idea in theory. You're addicted to some concepts that you've carried around for a long time. There is no value in requiring casts, they're just a funky smell, and force the user to perform potentially unsafe manual conversions, or interactions that they don't understand.
>
> For example (your example):
>
> struct NotThreadsafe
> {
>    private int x;
>    void local()
>    {
>      ++x; // <- invalidates the method below, you violate the other
> function's `shared` promise
>    }
>    void notThreadsafe() shared
>    {
>      atomicIncrement(&x);
>    }
> }
>
> First, note the comment. I can't look ONLY at the implementation of "notThreadSafe" (assuming the function name is less of a giveaway) in order to guarantee that it's actually thread safe. I have to look at the WHOLE MODULE. Anything could potentially do what local() does. I added private to x to at least give the appearance of thread safety.
>
> But on top of that, if I can't implicitly cast mutable to shared, then this ACTUALLY IS thread safe, as long as all the casting in the module is sound (easy to search and verify), and hopefully all the casting is encapsulated in primitives like you have written. Because someone on the outside would have to cast a mutable item into a shared item, and this puts the responsibility on them to make sure it works.
>
> I'm ALL FOR having shared be completely unusable as-is unless you cast (thanks for confirming what I suspected in your last post). It's the implicit casting which I think makes things way more difficult, and completely undercuts the utility of the compiler's mechanical checking.
>
> And on top of that, I WANT that implementation. If I know something is not shared, why would I ever want to use atomics on it? I don't like needlessly throwing away performance. This is how I would write it:
>
> struct ThreadSafe
> {
>     private int x;
>     void increment()
>     {
>        ++x; // I know this is not shared, so no reason to use atomics
>     }
>     void increment() shared
>     {
>        atomicIncrement(&x); // use atomics, to avoid races
>     }
> }
>
> The beauty of shared not being implicitly castable, is it allows you to focus on the implementation at hand, with the knowledge that nothing else can meddle with it. The goal of mechanical checking should be to narrow the focus of what needs to be proven correct.
>
> -Steve

I understand your argument, and I used to think this too... but I
concluded differently for 1 simple reason: usability.
I have demonstrated these usability considerations in production. I am
confident it's the right balance.

I propose:
 1. Normal people don't write thread-safety, a very small number of
unusual people do this. I feel very good about biasing 100% of the
cognitive load INSIDE the shared method. This means the expert, and
ONLY the expert, must make decisions about thread-safety
implementation.
 2. Implicit conversion allows users to safely interact with safe
things without doing unsafe casts. I think it's a complete design fail
if you expect any user anywhere to perform an unsafe cast to call a
perfectly thread-safe function. The user might not properly understand
their obligations.
 3. The practical result of the above is, any complexity relating to
safety is completely owned by the threadsafe author, and not cascaded
to the user. You can't expect users to understand, and make correct
decisions about threadsafety. Safety should be default position.

I recognise the potential loss of an unsafe optimised thread-local path.
1. This truly isn't a big deal. If this is really hurting you, you
will notice on the profiler, and deploy a thread-exclusive path
assuming the context supports it.
2. I will trade that for confidence in safe interaction every day of
the week. Safety is the right default position here.
2. You just need to make the unsafe thread-exclusive variant explicit, eg:

> struct ThreadSafe
> {
>     private int x;
>     void unsafeIncrement() // <- make it explicit
>     {
>        ++x; // User has asserted that no sharing is possible, no reason to use atomics
>     }
>     void increment() shared
>     {
>        atomicIncrement(&x); // object may be shared
>     }
> }

I think this is quiet a reasonable and clearly documented compromise. I think absolutely-reliably-threadsafe-by-default is the right default position. And if you want to accept unsafe operations for optimsation circumstances, then you're welcome to deploy that in your code as you see fit.

If the machinery is not a library for distribution and local to your
application, and you know for certain that your context is such that
thread-local and shared are mutually exclusive, then you're free to
make the unshared overload not-threadsafe; you can do this because you
know your application context.
You just shouldn't make widely distributed tooling this way.

I will indeed do this myself in some cases, because I know those facts
about my application.
But I wouldn't compromise the default design of shared for this
optimisation potential... deliberately deployed optimisation is okay
to be unsafe when taken in context.

On 10/18/18 1:47 PM, Stanislav Blinov wrote:
> On Thursday, 18 October 2018 at 17:17:37 UTC, Atila Neves wrote:
>> On Monday, 15 October 2018 at 18:46:45 UTC, Manu wrote:
>>> 1. shared should behave exactly like const, except in addition to inhibiting write access, it also inhibits read access.
>>
>> How is this significantly different from now?
>>
>> -----------------
>> shared int i;
>> ++i;
>>
>> Error: read-modify-write operations are not allowed for shared variables. Use core.atomic.atomicOp!"+="(i, 1) instead.
>> -----------------
>>
>> There's not much one can do to modify a shared value as it is.
> 
> i = 1;
> int x = i;
> shared int y = i;

This should be fine, y is not shared when being created.

However, this still is allowed, and shouldn't be:

y = 5;

-Steve

On Thursday, 18 October 2018 at 18:26:27 UTC, Steven Schveighoffer wrote:
> On 10/18/18 1:47 PM, Stanislav Blinov wrote:
>> On Thursday, 18 October 2018 at 17:17:37 UTC, Atila Neves wrote:
>>> On Monday, 15 October 2018 at 18:46:45 UTC, Manu wrote:
>>>> 1. shared should behave exactly like const, except in addition to inhibiting write access, it also inhibits read access.
>>>
>>> How is this significantly different from now?
>>>
>>> -----------------
>>> shared int i;
>>> ++i;
>>>
>>> Error: read-modify-write operations are not allowed for shared variables. Use core.atomic.atomicOp!"+="(i, 1) instead.
>>> -----------------
>>>
>>> There's not much one can do to modify a shared value as it is.
>> 
>> i = 1;
>> int x = i;
>> shared int y = i;
>
> This should be fine, y is not shared when being created.

'y' isn't, but 'i' is. It's fine on amd64, but that's incidental.

On Thu, Oct 18, 2018 at 6:50 AM Steven Schveighoffer via Digitalmars-d <digitalmars-d@puremagic.com> wrote:
>
> On 10/18/18 9:35 AM, Steven Schveighoffer wrote:
> >
> > struct NotThreadsafe
> > {
> >    private int x;
> >    void local()
> >    {
> >      ++x; // <- invalidates the method below, you violate the other
> > function's `shared` promise
> >    }
> >    void notThreadsafe() shared
> >    {
> >      atomicIncrement(&x);
> >    }
> > }
> >
>
> [snip]
>
> > But on top of that, if I can't implicitly cast mutable to shared, then this ACTUALLY IS thread safe, as long as all the casting in the module is sound (easy to search and verify), and hopefully all the casting is encapsulated in primitives like you have written. Because someone on the outside would have to cast a mutable item into a shared item, and this puts the responsibility on them to make sure it works.
> >
>
> Another thing to point out -- I can make x public (not private), and
> it's STILL THREAD SAFE.

I'm not sure that's an interesting design goal though.
Most things don't have shared methods. If you're writing a thing with
shared methods, you're very in the business of implementing
threadsafety... you're going to want to make it the tightest,
most-unlikely-to-have-threading-bugs thing you can write.
I predict that you're not going to be upset about the restriction.

And if you are, AND you're confident in your application to maintain a
mutually-exclusive shared/TL separation, then you can do this to your
hearts content! Nobody will stop you, and it will be fine.
But I don't think it should be default, because the rules as designed
that way, enforce *users* to perform unsafe casts when they're, on
average, not qualified to make those decisions.

On Thu, Oct 18, 2018 at 7:20 AM Steven Schveighoffer via Digitalmars-d <digitalmars-d@puremagic.com> wrote:
>
> On 10/18/18 10:11 AM, Simen Kjærås wrote:
> > On Thursday, 18 October 2018 at 13:35:22 UTC, Steven Schveighoffer wrote:
> >> struct ThreadSafe
> >> {
> >>    private int x;
> >>    void increment()
> >>    {
> >>       ++x; // I know this is not shared, so no reason to use atomics
> >>    }
> >>    void increment() shared
> >>    {
> >>       atomicIncrement(&x); // use atomics, to avoid races
> >>    }
> >> }
> >
> > But this isn't thread-safe, for the exact reasons described elsewhere in this thread (and in fact, incorrectly leveled at Manu's proposal). Someone could write this code:
> >
> > void foo() {
> >      ThreadSafe* a = new ThreadSafe();
> >      shareAllOver(a);
>
> Error: cannot call function shareAllOver(shared(ThreadSafe) *) with type
> ThreadSafe *

And here you expect a user to perform an unsafe-cast (which they may
not understand), and we have no language semantics to enforce the
transfer of ownership. How do you assure that the user yields the
thread-local instance?
I think requiring the cast is un-principled in every way that D values.

My proposal doesn't rely on convention (that we *hope* the user does correctly yield the thread-local instance)... it assures a set of safe rules by default.

This is the core value proposition of my proposal. It's literally the entire point.

On Thu, Oct 18, 2018 at 7:20 AM Steven Schveighoffer via Digitalmars-d <digitalmars-d@puremagic.com> wrote:
>
> On 10/18/18 10:11 AM, Simen Kjærås wrote:
> >      a.increment(); // unsafe, non-shared method call
> > }
> >
> > When a.increment() is being called, you have no idea if anyone else is
> > using the shared interface.
>
> I do, because unless you have cast the type to shared, I'm certain there is only thread-local aliasing to it.

No, you can never be sure. Your assumption depends on the *user* engaging in an unsafe operation (the cast), and correctly perform a conventional act; they must correctly the safely transfer ownership. My proposal puts all requirements on the author, not the user. I think this is a much more trustworthy relationship, and in terms of cognitive load, author:users is a 1:many relationship, and I place the load on the '1', not the 'many.

On Thursday, 18 October 2018 at 18:24:47 UTC, Manu wrote:

> I have demonstrated these usability considerations in production. I am
> confident it's the right balance.

Then convince us. So far you haven't.

> I propose:
>  1. Normal people don't write thread-safety, a very small number of
> unusual people do this. I feel very good about biasing 100% of the
> cognitive load INSIDE the shared method. This means the expert, and
> ONLY the expert, must make decisions about thread-safety implementation.

No argument.

>  2. Implicit conversion allows users to safely interact with safe
> things without doing unsafe casts. I think it's a complete design fail
> if you expect any user anywhere to perform an unsafe cast to call a
> perfectly thread-safe function. The user might not properly understand
> their obligations.

Disagreed. "Normal" people wouldn't be doing any unsafe casts and *must not be able to* do something as unsafe as silent promotion of thread-local mutable data to shared. But it's perfectly fine for the "expert" user to do such a promotion, explicitly.

>  3. The practical result of the above is, any complexity relating to
> safety is completely owned by the threadsafe author, and not cascaded
> to the user. You can't expect users to understand, and make correct
> decisions about threadsafety. Safety should be default position.

Exactly. And an implicit conversion from mutable to shared isn't safe at all.

> I recognise the potential loss of an unsafe optimised thread-local path.
> 1. This truly isn't a big deal. If this is really hurting you, you
> will notice on the profiler, and deploy a thread-exclusive path
> assuming the context supports it.

> 2. I will trade that for confidence in safe interaction every day of
> the week. Safety is the right default position here.

Does not compute. Either you're an "expert" from above and live with that burden, or you're not.

> 2. You just need to make the unsafe thread-exclusive variant explicit, eg:
>
>> struct ThreadSafe
>> {
>>     private int x;
>>     void unsafeIncrement() // <- make it explicit
>>     {
>>        ++x; // User has asserted that no sharing is possible, no reason to use atomics
>>     }
>>     void increment() shared
>>     {
>>        atomicIncrement(&x); // object may be shared
>>     }
>> }

No. The above code is not thread-safe at all. The private int *must* be declared shared. Then it becomes:

struct ThreadSafe {
    // These must be *required* if you want to assert any thread safety. Be nice
    // if the compiler did that for us.
    @disable this(this);
    @disable void opAssign(typeof(this));

    private shared int x; // <- *must* be shared

    void unsafeIncrement() @system {
        x.assumeUnshared += 1;
    }

    // or deduced:

    void unsafeIncrement()() // assumeUnshared must be @system, thus this unsafeIncrement will also be deduced @system
    {
        x.assumeUnshared += 1;
    }

    void increment() shared {
        x.atomicOp!"+="(1);
    }
}

> I think this is quiet a reasonable and clearly documented compromise.

With the fixes above, it is. Without them, it will only be apparent from documentation, and who writes, or reads, that?..

> I think absolutely-reliably-threadsafe-by-default is the right default position.

But it is exactly the opposite of automatic promotions from mutable to shared!

On Thursday, 18 October 2018 at 17:47:29 UTC, Stanislav Blinov wrote:
> On Thursday, 18 October 2018 at 17:17:37 UTC, Atila Neves wrote:
>> On Monday, 15 October 2018 at 18:46:45 UTC, Manu wrote:
>>> Assuming the rules above: "can't read or write to members", and the understanding that `shared` methods are expected to have threadsafe implementations (because that's the whole point), what are the risks from allowing T* -> shared(T)* conversion?
>>
>> int i;
>> tid.send(&i);
>> ++i;  // oops, data race
>
> Doesn't work. No matter what you show Manu or Simen here they think it's just a bad contrived example. You can't sway them by the fact that the compiler currently *prevents* this from happening.

Manu said clearly that the receiving thread won't be able to read or write the pointer.

Because int or int* does not have threadsafe member functions.


You can still disagree on the merits, but so far it has been demonstrated as a sound idea.