shared - i need it to be useful

[Manu]

On Mon, Oct 15, 2018 at 4:35 PM Stanislav Blinov via Digitalmars-d
<digitalmars-d at puremagic.com> wrote:
>
> On Monday, 15 October 2018 at 21:51:43 UTC, Manu wrote:
>
> > If a shared method is incompatible with an unshared method,
> > your class is broken.
>
> What?!? So... my unshared methods should also perform all that's
> necessary for `shared` methods?

Of course! You're writing a threadsafe object... how could you expect otherwise?

> > Explicit casting doesn't magically implement thread-safety, it
> > basically just guarantees failure.
>
> It doesn't indeed. It does, however, at least help prevent silent
> bugs. Via that same guaranteed failure. That failure is about all
> the help we can get from the compiler anyway.

Just to be clear, what I'm suggesting is a significant *restriction*
to what shared already does... there will be a whole lot more safety
under my proposal.
The cast gives exactly nothing that attributing a method as shared
doesn't give you, except that attributing a method shared is so much
more sanitary and clearly communicates intent at the API level.

> > What I suggest are rules that lead to proper behaviour with
> > respect to writing a thread-safe API.
> > You can write bad code with any feature in any number of ways.
>
> Yup. For example, passing an int* to a function expecting shared
> int*.

I don't understand your example. What's the problem you're suggesting?

> > I see it this way:
> > If your object has shared methods, then it is distinctly and
> > *deliberately* involved in thread-safety. You have deliberately
> > opted-in to writing a thread-safe object, and you must deliver
> > on your promise.
>
> > The un-shared API of an object that supports `shared` are not
> > exempt from the thread-safety commitment, they are simply the
> > subset of the API that may not be called from a shared context.
>
> And therefore they lack any synchronization. So I don't see how
> they *can* be "compatible" with `shared` methods.

I don't understand this statement either. Who said they lack
synchronisation? If they need it, they will have it.
There's a good chance they don't need it though, they might not
interact with a thread-unsafe portion of the class.

> > If your shared method is incompatible with other methods, your
> > class is broken, and you violate your promise.
>
> Nope.

So certain...

> class BigCounter {
>
>      this() { /* don't even need the mutex if I'm not sharing this
> */ }
>
>      this(Mutex m = null) shared {
>          this.m = m ? m : new Mutex;
>      }
>
>      void increment() { value += 1; }
>      void increment() shared { synchronized(m)
> *value.assumeUnshared += 1; }
>
> private:
>      Mutex m;
>      BigInt value;
> }

You've just conflated 2 classes into one. One is a threadlocal
counter, the other is a threadsafe counter. Which is it?
Like I said before: "you can contrive a bad program with literally any
language feature!"

> They're not "compatible" in any shape or form.

Correct, you wrote 2 different things and mashed them together.

> Or would you have
> the unshared ctor also create the mutex and unshared increment
> also take the lock? What's the point of having them  then? Better
> disallow mixed implementations altogether (which is actually not
> that bad of an idea).

Right. This is key to my whole suggestion. If you write a shared
thing, you accept that it's shared! You don't just accept it, you jam
the stake in the ground.
There's a relatively small number of things that need to be
threadsafe, you won't see `shared` methods appearing at random. If you
use shared, you promise threadsafety OR the members of the thing are
inaccessible without some sort of lock-&-cast-away treatment.

> > Nobody writes methods of an object such that they don't work
> > with each other... methods are part of a deliberately crafted
> > and packaged
> > entity. If you write a shared object, you do so deliberately,
> > and you buy responsibility of making sure your objects API is
> > thread-safe.
> > If your object is not thread-safe, don't write shared methods.
>
> Ahem... Okay...
>
> import std.concurrency;
> import core.atomic;
>
> void thread(shared int* x) {
>      (*x).atomicOp!"+="(1);
> }
>
> shared int c;
>
> void main() {
>      int x;
>      auto tid = spawn(&thread, &x); // "just" a typo
> }
>
> You're saying that's ok, it should "just" compile. It shouldn't.
> It should produce an error and a mild electric discharge into the
> developer's chair.

Yup. It's a typo. You passed a stack pointer to a scope that outlives
the caller.
That class of issue is not on trial here. There's DIP1000, and all
sorts of things to try and improve safety in terms of lifetimes.
You only managed to contrive this by spawning a thread. If it were
just a normal function, this would be perfectly legitimate, and again,
that's my whole point.