On Wednesday, July 19, 2017 8:59:03 PM MDT Atila Neves via Digitalmars-d wrote:
> On Wednesday, 19 July 2017 at 20:23:18 UTC, Jonathan M Davis
>
> wrote:
> > On Wednesday, July 19, 2017 2:29:04 PM MDT Atila Neves via
> >
> > Digitalmars-d wrote:
> >> On Tuesday, 18 July 2017 at 19:24:18 UTC, Jonathan M Davis
> >>
> >> wrote:
> >> > On Tuesday, July 18, 2017 18:06:56 Atila Neves via
> >> >
> >> >> Except for a programmer explicitly and manually calling the destructor (in which case, don't), the destructor is only ever called by one thread.
> >> >
> >> > It could still be a problem if the struct has a member variable that is a reference type, because then something else could refer to that object, and if it's shared, then you would need to protect it, and the operations that shared prevents should still be prevented. For full-on value types, it should be a non-issue though.
> >> >
> >> > - Jonathan M Davis
> >>
> >> Mmm, I guess so. As Marco pointed out, it's a similar problem with immutable (because the compiler casts it away before calling the destructor).
> >>
> >> Although I dare say that anybody writing code that depends on such locking and destruction when shared is unlikely to get it right in the first place.
> >
> > Well, consider that something like a reference counted type would have to get this right. Now, a reference counted type that worled with shared would need to be written that way - simply slapping shared on such a smart pointer type isn't going to work - but it would then be really bad for the destructor to be treated as thread-local.
>
> Not necessarily - the reference counted smart pointer doesn't have to be `shared` itself to have a `shared` payload. I'm not even entirely sure what the advantage of it being `shared` would be, or even what that would really mean.
>
> The way `automem.RefCounted` works right now is by doing atomic reference count manipulations by using reflection to know if the payload is `shared`.

Okay, but my point is that it's perfectly legal to have a shared object that refers to other shared objects that it does not own, and casting away shared in the destructor means that the compiler can no longer enforce shared like it's supposed to.

> > It really looks to me like having a thread-local dstructor for shared data is just begging for problems. It may work in the simple cases, but it'll fall apart in the more complicated ones, and I don't see how that's acceptable.
>
> You've definitely made me wonder about complicated cases, but I'd argue that they'd be rare. Destructors are (bar manually calling them) run in one thread. I'm having trouble imagining a situation where two threads have references to a `shared` object/value that is going to be destroyed deterministically.

The issue isn't the object being destroyed. It's what it refers to via its member variables. For instance, what if an object were to remove itself from a shared list when it's destroyed (e.g. because it's an observer in the observer pattern). The object has a reference to the list, but it doesn't own it. So, the fact that the destructor is only run in a single thread doesn't help any. You could have ten of these objects all being destroyed and accessing the same list at the same time from different threads, and if the destructor treats the objects as thread-local - and thus treats all of the member variables as thread-local, then you won't get the compiler errors that you're supposed to get when you do something non-atomic with shared.

Sure, the list could be designed in such a way that it protects itself against threading issues, but it's perfectly legal to have it be shared and require that anyone using it lock a mutex, and destructors need to take that into account just like any other function would.

Basically, what these changes have done is act like all destructors are part of a synchronized class (as described in TDPL) where shared is stripped off of the member variables - except that _all_ of shared has been stripped off instead of just the outer layer, but these aren't synchronized classes, and they don't provide any guarantees about references not escaping or locking occurring, and not even synchronized classes can safely cast away shared except for the outer layer. As such, from what I can tell, these changes are very broken. Yes, we need a solution which allows us to deal with shared destructors properly, but casting away shared without any guarantees beyond the fact that no other threads are calling any member functions on that object at that time simply does not guarantee that shared objects have been properly protected. To do that, you'd essentially need synchronized classes, and even they can only strip off the outer layer of shared, not the whole thing. Yes, destructors have fewer problems with shared than most functions, but there's nothing about then which makes them immune to issues with shared.

- Jonathan M Davis

On Wednesday, 19 July 2017 at 22:35:43 UTC, Jonathan M Davis wrote:
> The issue isn't the object being destroyed. It's what it refers to via its member variables. For instance, what if an object were to remove itself from a shared list when it's destroyed (e.g. because it's an observer in the observer pattern). The object has a reference to the list, but it doesn't own it.
So, even a thread-local object that has references to a shared list
has to handle those as shared, even in its non-shared destructor.
I can't follow your argument.

On Wednesday, 19 July 2017 at 12:56:38 UTC, Marco Leise wrote:
> That's exactly what I was opposing in the other post. These
> handles are opaque and never change their value. Within the
> Dlang language barrier they can be immutable and as such,
> implicitly shared.

Given transitivity of immutability the handle should have the same immutability as the resource it represents.

> It sometimes depends on whether a library was
> compiled with multi-threading support or not

Then you can communicate multithreading support with type system.

> and a value type can be copied from and to shared anyways, rendering the safety
> argument void:
>
> 	int x;
> 	shared int y = x;
> 	int z = y;

If it's overlooked, it doesn't mean D can't have proper sharing.

On Wednesday, 19 July 2017 at 20:59:03 UTC, Atila Neves wrote:
> Not necessarily - the reference counted smart pointer doesn't have to be `shared` itself to have a `shared` payload.

Yes, but it can be done either way. It's actually what Jack is trying to do: make stdout shared and reference counted: https://issues.dlang.org/show_bug.cgi?id=15768#c7

> I'm not even entirely sure what the advantage of it being `shared` would be, or even what that would really mean.

It will be thread safe and its lifetime will be automatically managed.

> You've definitely made me wonder about complicated cases, but I'd argue that they'd be rare. Destructors are (bar manually calling them) run in one thread. I'm having trouble imagining a situation where two threads have references to a `shared` object/value that is going to be destroyed deterministically.

A mutex, a file, a socket, any shareable resource. Though I agree that reference counting of shared resources should be optimized by thread local counters.

On Wednesday, 19 July 2017 at 21:50:32 UTC, Petar Kirov [ZombineDev] wrote:
> Note that this doesn't play well with regular [1] value types becuase e.g. you don't have control over the synthesized bit-blit for this(this) and so you can't assume that structs with a single pointer member are updated atomically, even if would write the opAssign that way. In C++17 atomic_shared_ptr has it's copy-constructor and assign operator deleted. You can only do atomic<T> like ops with it and derive a plain shared_ptr<T> from it, kind-of like core.atomic's HeadUnshared(T).

Huh? Why opAssign can't just do what atomic<T> does?

On Thursday, 20 July 2017 at 07:40:35 UTC, Dominikus Dittes Scherkl wrote:
> On Wednesday, 19 July 2017 at 22:35:43 UTC, Jonathan M Davis wrote:
>> The issue isn't the object being destroyed. It's what it refers to via its member variables. For instance, what if an object were to remove itself from a shared list when it's destroyed (e.g. because it's an observer in the observer pattern). The object has a reference to the list, but it doesn't own it.
> So, even a thread-local object that has references to a shared list
> has to handle those as shared, even in its non-shared destructor.
> I can't follow your argument.

Thread local object can't be contained in a shared list, the list is referred as unqualified, and thread local object will be contained in a thread local list, and shared object will be contained in a shared list because of transitivity of the shared qualifier.

On Thursday, 20 July 2017 at 10:16:21 UTC, Kagamin wrote:
> On Wednesday, 19 July 2017 at 21:50:32 UTC, Petar Kirov [ZombineDev] wrote:
>> Note that this doesn't play well with regular [1] value types becuase e.g. you don't have control over the synthesized bit-blit for this(this) and so you can't assume that structs with a single pointer member are updated atomically, even if would write the opAssign that way. In C++17 atomic_shared_ptr has it's copy-constructor and assign operator deleted. You can only do atomic<T> like ops with it and derive a plain shared_ptr<T> from it, kind-of like core.atomic's HeadUnshared(T).
>
> Huh? Why opAssign can't just do what atomic<T> does?

opAssign is fine, the problem is with the this(this).

On Thursday, 20 July 2017 at 10:16:21 UTC, Kagamin wrote:
> On Wednesday, 19 July 2017 at 21:50:32 UTC, Petar Kirov [ZombineDev] wrote:
>> Note that this doesn't play well with regular [1] value types becuase e.g. you don't have control over the synthesized bit-blit for this(this) and so you can't assume that structs with a single pointer member are updated atomically, even if would write the opAssign that way. In C++17 atomic_shared_ptr has it's copy-constructor and assign operator deleted. You can only do atomic<T> like ops with it and derive a plain shared_ptr<T> from it, kind-of like core.atomic's HeadUnshared(T).
>
> Huh? Why opAssign can't just do what atomic<T> does?

Also note that atomic<T> doesn't have neither copy constructor nor assignment operator:

http://en.cppreference.com/w/cpp/atomic/atomic/atomic

> atomic( const atomic& ) = delete;

http://en.cppreference.com/w/cpp/atomic/atomic/operator%3D

> atomic& operator=( const atomic& ) = delete;
> atomic& operator=( const atomic& ) volatile = delete;

On Thursday, 20 July 2017 at 10:19:30 UTC, Kagamin wrote:
> On Thursday, 20 July 2017 at 07:40:35 UTC, Dominikus Dittes Scherkl wrote:
>> On Wednesday, 19 July 2017 at 22:35:43 UTC, Jonathan M Davis wrote:
>>> The issue isn't the object being destroyed. It's what it refers to via its member variables. For instance, what if an object were to remove itself from a shared list when it's destroyed (e.g. because it's an observer in the observer pattern). The object has a reference to the list, but it doesn't own it.
>> So, even a thread-local object that has references to a shared list
>> has to handle those as shared, even in its non-shared destructor.
>> I can't follow your argument.
>
> Thread local object can't be contained in a shared list, the list is referred as unqualified, and thread local object will be contained in a thread local list, and shared object will be contained in a shared list because of transitivity of the shared qualifier.

It's the other way around:

ThreadLocal tl;

struct ThreadLocal
{
    shared(ListNode*)* listHead;
    shared(ListNode)*  listNode;

    ~this()
    {
        listHead.removeNodeFromList(listNode);
    }
}

On Thursday, July 20, 2017 07:40:35 Dominikus Dittes Scherkl via Digitalmars-d wrote:
> On Wednesday, 19 July 2017 at 22:35:43 UTC, Jonathan M Davis
>
> wrote:
> > The issue isn't the object being destroyed. It's what it refers to via its member variables. For instance, what if an object were to remove itself from a shared list when it's destroyed (e.g. because it's an observer in the observer pattern). The object has a reference to the list, but it doesn't own it.
>
> So, even a thread-local object that has references to a shared
> list
> has to handle those as shared, even in its non-shared destructor.
> I can't follow your argument.

You can't just strip off shared. To do so defeats the purpose of shared. If you have something like

struct S
{
     shared List<Foo> _list;

     ~this()
     {
        ...
     }
}

then inside of the destructor, _list is not treated as shared, meaning that none of the compiler protections for shared are in place, no locking has occurred, and the compiler is free to make optimizations based on the wrong assumption that all of the member variables are thread-local. If nothing else has access to that list, then it'll work, but if anything else does - and if it's a reference type, that's perfectly possible - then you have a threading problem, because shared has been violated.

Except in cases where the member variables are all value types and thus no other references to them should exist when the destructor is called, stripping away shared from them means that the compiler can no longer properly enforce shared, and it's going to make the wrong assumptions about whether the data can be treated as thread-local or not.

If we go with the assumption that nothing has pointers to the member variables (since doing so would be @system, and they're only valid so long as the struct isn't moved anyway), you can probably strip off the outer layer of shared safely in the destructor, but if you're dealing with a reference type, anything it points to needs to still be treated as shared.

- Jonathan M Davis