www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - Assignment of shared values

reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
I'd submitted https://issues.dlang.org/show_bug.cgi?id=4130 which is 
related to issues with shared values assignment.

Consider:

struct S { long a, b, c; ... }

Should it be safe to assign one S to another? I guess not because 
assignment would violate whatever invariant a, b, and c may hold, and 
there's too much state to be assigned atomically. Somewhat sadly, this 
code does compile:

shared S s1, s2;
s1 = s2;

However, assigning Variant objects holding such structs does not 
compile, which in turn leads to the reported bug in std.concurrency. Now 
it seems to me that the only way is to adapt Variant to allow such 
assignments, otherwise we'd be breaking existing code. Thoughts appreciated.


Andrei
Aug 06 2014
next sibling parent reply "Brad Anderson" <eco gnuk.net> writes:
On Wednesday, 6 August 2014 at 23:01:11 UTC, Andrei Alexandrescu 
wrote:
 I'd submitted https://issues.dlang.org/show_bug.cgi?id=4130 
 which is related to issues with shared values assignment.
Did you mean https://issues.dlang.org/show_bug.cgi?id=13262 ?
Aug 06 2014
parent Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 8/6/14, 4:06 PM, Brad Anderson wrote:
 On Wednesday, 6 August 2014 at 23:01:11 UTC, Andrei Alexandrescu wrote:
 I'd submitted https://issues.dlang.org/show_bug.cgi?id=4130 which is
 related to issues with shared values assignment.
Did you mean https://issues.dlang.org/show_bug.cgi?id=13262 ?
Yes, sorry and thanks. -- Andrei
Aug 06 2014
prev sibling next sibling parent "Mike" <none none.com> writes:
On Wednesday, 6 August 2014 at 23:01:11 UTC, Andrei Alexandrescu 
wrote:
 Now it seems to me that the only way is to adapt Variant to 
 allow such assignments, otherwise we'd be breaking existing 
 code. Thoughts appreciated.
I'm still of the opinion that if the change fixes a design flaw in the language or breaks code that should not have been allowed in the first place, breakage is justified. This no-breaking-changes-allowed trend is preventing D from reaching its potential. In this situation, please break away (pun intended). Mike
Aug 06 2014
prev sibling next sibling parent reply "Dicebot" <public dicebot.lv> writes:
On Wednesday, 6 August 2014 at 23:01:11 UTC, Andrei Alexandrescu 
wrote:
 Somewhat sadly, this code does compile:

 shared S s1, s2;
 s1 = s2;
I have no idea what semantics this may have from just reading the snippet. IMHO killing it will be a good thing and totally in line with your comments about shared you have made during DConf conversations.
 However, assigning Variant objects holding such structs does 
 not compile, which in turn leads to the reported bug in 
 std.concurrency. Now it seems to me that the only way is to 
 adapt Variant to allow such assignments, otherwise we'd be 
 breaking existing code. Thoughts appreciated.
I think biggest std.concurrency problem is that people try to use `shared` as poor replacement to `unique`/`isolated` and this is not what it is supposed to mean. Making `Unique` work with std.concurrency can be a good direction to make things much less confusing.
Aug 07 2014
next sibling parent reply "Marc =?UTF-8?B?U2Now7x0eiI=?= <schuetzm gmx.net> writes:
On Thursday, 7 August 2014 at 12:25:09 UTC, Dicebot wrote:
 On Wednesday, 6 August 2014 at 23:01:11 UTC, Andrei 
 Alexandrescu wrote:
 Somewhat sadly, this code does compile:

 shared S s1, s2;
 s1 = s2;
I have no idea what semantics this may have from just reading the snippet. IMHO killing it will be a good thing and totally in line with your comments about shared you have made during DConf conversations.
Should it be allowed if `S` defines `opAssign`?
Aug 07 2014
parent "Idan Arye" <GenericNPC gmail.com> writes:
On Thursday, 7 August 2014 at 12:45:12 UTC, Marc Sch├╝tz wrote:
 On Thursday, 7 August 2014 at 12:25:09 UTC, Dicebot wrote:
 On Wednesday, 6 August 2014 at 23:01:11 UTC, Andrei 
 Alexandrescu wrote:
 Somewhat sadly, this code does compile:

 shared S s1, s2;
 s1 = s2;
I have no idea what semantics this may have from just reading the snippet. IMHO killing it will be a good thing and totally in line with your comments about shared you have made during DConf conversations.
Should it be allowed if `S` defines `opAssign`?
Only if `S` defines a shared `opAssign`. In matter of fact, if `S` defines a non-shared `opAssign` without defining the shared version that code won't compile(which is probably why the `Variant` case doesn't compile).
Aug 07 2014
prev sibling next sibling parent reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 8/7/14, 5:25 AM, Dicebot wrote:
 On Wednesday, 6 August 2014 at 23:01:11 UTC, Andrei Alexandrescu wrote:
 Somewhat sadly, this code does compile:

 shared S s1, s2;
 s1 = s2;
I have no idea what semantics this may have from just reading the snippet.
Well right now it just copies field by field, non-atomically.
 IMHO killing it will be a good thing and totally in line with
 your comments about shared you have made during DConf conversations.
Yah. The basic intent behind "shared" is to disallow unwitting racy code, and allow users to define robust shared abstractions. Consider this code at top level: struct S { private long a, b, c; ... } shared S g_theS; All threads will "see" the same g_theS. The intent of "shared" is to disallow racy/wrong use of g_theS across threads. However, assigning g_theS is by default allowed and does something definitely racy. So I think we should do the following: * if S.sizeof <= size_t.sizeof, generate atomic assignment automatically for shared values. * if S.sizeof == 2 * size_t.sizeof, generate special 128-bit atomic assignment for shared values on 64-bit systems, and 64-bit atomic assignment on 32-bit systems. * In all other cases, reject code during compilation and require an opAssign for shared values. This will break code. It may even break correct code (benign races or code that uses external locking). On the other hand, it seems like the right thing to do by the intended semantics of "shared".
 However, assigning Variant objects holding such structs does not
 compile, which in turn leads to the reported bug in std.concurrency.
 Now it seems to me that the only way is to adapt Variant to allow such
 assignments, otherwise we'd be breaking existing code. Thoughts
 appreciated.
I think biggest std.concurrency problem is that people try to use `shared` as poor replacement to `unique`/`isolated` and this is not what it is supposed to mean. Making `Unique` work with std.concurrency can be a good direction to make things much less confusing.
Agreed. Andrei
Aug 07 2014
parent reply "Peter Alexander" <peter.alexander.au gmail.com> writes:
On Thursday, 7 August 2014 at 16:44:37 UTC, Andrei Alexandrescu 
wrote:
 Yah. The basic intent behind "shared" is to disallow unwitting 
 racy code, and allow users to define robust shared abstractions.

 Consider this code at top level:

 struct S {
   private long a, b, c;
   ...
 }
 shared S g_theS;

 All threads will "see" the same g_theS. The intent of "shared" 
 is to disallow racy/wrong use of g_theS across threads. 
 However, assigning g_theS is by default allowed and does 
 something definitely racy.

 So I think we should do the following:

 * if S.sizeof <= size_t.sizeof, generate atomic assignment 
 automatically for shared values.

 * if S.sizeof == 2 * size_t.sizeof, generate special 128-bit 
 atomic assignment for shared values on 64-bit systems, and 
 64-bit atomic assignment on 32-bit systems.

 * In all other cases, reject code during compilation and 
 require an opAssign for shared values.

 This will break code. It may even break correct code (benign 
 races or code that uses external locking). On the other hand, 
 it seems like the right thing to do by the intended semantics 
 of "shared".
Even with that change, you'll still be able to do: S otherS = ...; g_theS.a = otherS.a; g_theS.b = otherS.b; g_theS.c = otherS.c; Right? This violates the same invariants as: g_theS = otherS; Can you explain the practical benefits of enforcing atomicity of shared objects only at the individual statement level? I must be missing something.
Aug 07 2014
parent Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 8/7/14, 11:22 AM, Peter Alexander wrote:
 On Thursday, 7 August 2014 at 16:44:37 UTC, Andrei Alexandrescu wrote:
 Yah. The basic intent behind "shared" is to disallow unwitting racy
 code, and allow users to define robust shared abstractions.

 Consider this code at top level:

 struct S {
   private long a, b, c;
   ...
 }
 shared S g_theS;

 All threads will "see" the same g_theS. The intent of "shared" is to
 disallow racy/wrong use of g_theS across threads. However, assigning
 g_theS is by default allowed and does something definitely racy.

 So I think we should do the following:

 * if S.sizeof <= size_t.sizeof, generate atomic assignment
 automatically for shared values.

 * if S.sizeof == 2 * size_t.sizeof, generate special 128-bit atomic
 assignment for shared values on 64-bit systems, and 64-bit atomic
 assignment on 32-bit systems.

 * In all other cases, reject code during compilation and require an
 opAssign for shared values.

 This will break code. It may even break correct code (benign races or
 code that uses external locking). On the other hand, it seems like the
 right thing to do by the intended semantics of "shared".
Even with that change, you'll still be able to do: S otherS = ...; g_theS.a = otherS.a; g_theS.b = otherS.b; g_theS.c = otherS.c; Right? This violates the same invariants as:
Not if they're private. Access to non-encapsulated data is not supposed to be regulated by the language.
 g_theS = otherS;

 Can you explain the practical benefits of enforcing atomicity of shared
 objects only at the individual statement level? I must be missing
 something.
The whole idea behind shared is disallowing implicit races, a common source of bugs. All shared data is assumed to be visible to several threads at once, and as such is subject to additional restrictions compared to non-shared data, all of which is considered thread-local. That's why e.g. recently RMW operations on shared integrals were disallowed. Andrei
Aug 07 2014
prev sibling parent "Sean Kelly" <sean invisibleduck.org> writes:
On Thursday, 7 August 2014 at 12:25:09 UTC, Dicebot wrote:
 I think biggest std.concurrency problem is that people try to 
 use `shared` as poor replacement to `unique`/`isolated` and 
 this is not what it is supposed to mean. Making `Unique` work 
 with std.concurrency can be a good direction to make things 
 much less confusing.
+1
Aug 07 2014
prev sibling parent reply "Sean Kelly" <sean invisibleduck.org> writes:
On Wednesday, 6 August 2014 at 23:01:11 UTC, Andrei Alexandrescu 
wrote:
 I'd submitted https://issues.dlang.org/show_bug.cgi?id=4130 
 which is related to issues with shared values assignment.

 Consider:

 struct S { long a, b, c; ... }

 Should it be safe to assign one S to another? I guess not 
 because assignment would violate whatever invariant a, b, and c 
 may hold, and there's too much state to be assigned atomically.
But isn't it required that structs allows bit copying? Perhaps it's simply invalid to have a shared struct at all? Which in turn suggests that naive transitivity of shared isn't correct.
Aug 07 2014
parent Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 8/7/14, 10:12 AM, Sean Kelly wrote:
 On Wednesday, 6 August 2014 at 23:01:11 UTC, Andrei Alexandrescu wrote:
 I'd submitted https://issues.dlang.org/show_bug.cgi?id=4130 which is
 related to issues with shared values assignment.

 Consider:

 struct S { long a, b, c; ... }

 Should it be safe to assign one S to another? I guess not because
 assignment would violate whatever invariant a, b, and c may hold, and
 there's too much state to be assigned atomically.
But isn't it required that structs allows bit copying? Perhaps it's simply invalid to have a shared struct at all? Which in turn suggests that naive transitivity of shared isn't correct.
Shared structs should be allowed, operations on them should follow the usual qualifier checks, and transitivity should follow the usual qualifier rules. The problem at hand is assignment of shared structs is allowed - it shouldn't. Andrei
Aug 07 2014