www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - implicit conversions to/from shared

reply ag0aep6g <anonymous example.com> writes:
While messing with atomicLoad [1], I noticed that dmd lets me implicitly 
convert values to/from shared without restrictions. It's in the spec 
[2]. This seems bad to me.

Here's how I understand shared. If anything's wrong, I'd appreciate if 
someone would educate me.

Shared is there to prevent me from accidentally writing code that's not 
thread-safe (at a low level).

When I never use shared (or __gshared or other mean hacks), I only ever 
get thread-local variables and I can't share data between threads. I get 
thread safety by avoiding the issue entirely.

When I do use shared, the compiler is supposed to reject (low-level) 
operations that are not thread-safe. That's why ++x is deprecated for a 
shared x, and I'm told to use atomicOp!"+=" instead.

Of course I can still do a verbose unsafe version which the compiler 
can't catch:

     atomicStore(x, atomicLoad(x) + 1);

The compiler can't possibly know which high-level operations need to run 
uninterrupted, so there's nothing it can do about this.

Back to implicit conversions. Loading and storing is as low-level as it 
gets. When D disallows unsafe ++ on shared, surely it should also 
disallow unsafe loading and storing.

Copying a shared value type to an unshared one could be defined as doing 
an atomic load, and copying the other way around could result in an 
atomic store. I don't think it's specified or implemented like this at 
the moment, but it would make sense to me. Loading/storing overly large 
value types can probably not be made thread-safe like that.

So, conclusion/proposal:

* Conversion of a value type from shared to unshared should only be 
allowed if it can be done with an atomic load. The compiler should 
generate the atomic load.
* Conversion of a value type from unshared to shared should only be 
allowed if it can be done with an atomic store. The compiler should 
generate the atomic store.
* Other conversions are not allowed.

Or alternatively, simply disallow all those conversions, and tell the 
user that they have to ensure thread safety themselves, e.g. by using 
atomicLoad/atomicStore.

Sorry for the noise if this is all known and part of the "shared isn't 
implemented" mantra. I couldn't find a bugzilla issue.


[1] https://github.com/dlang/druntime/pull/1605
[2] https://dlang.org/spec/const3.html#implicit_conversions (first sentence)
Jul 10 2016
next sibling parent reply Alex Parrill <initrd.gz gmail.com> writes:
On Sunday, 10 July 2016 at 13:02:17 UTC, ag0aep6g wrote:

 While messing with atomicLoad [1], I noticed that dmd lets me 
 implicitly convert values to/from shared without restrictions. 
 It's in the spec [2]. This seems bad to me.

 [...]


Atomic loading and storing, from what I understand, is usually 
limited to about a word on most architectures. I don't think it 
would be good to implicitly define assignment and referencing as 
atomic operations, since it would be limited. IMO concurrent 
access is better off being explicit anyway.

I don't think there is an issue with converting unshared 
reference types to shared (ex. ref T -> ref shared(T) or T* -> 
shared(T)*); worst you get is some extra synchronization.
Jul 10 2016
parent reply ag0aep6g <anonymous example.com> writes:
On 07/11/2016 01:52 AM, Alex Parrill wrote:

 Atomic loading and storing, from what I understand, is usually limited
 to about a word on most architectures. I don't think it would be good to
 implicitly define assignment and referencing as atomic operations, since
 it would be limited. IMO concurrent access is better off being explicit
 anyway.


Simply disallow reading and writing shared then, forcing something more 
explicit like atomicLoad/atomicStore?

That would be better than the current state, but it would make shared 
even more unwieldy.

Generating atomic operations would break less code, and feels like the 
obvious thing to me. It would be kinda explicit: shared being involved 
gives it away. Also, with other forms of ensuring thread safety, you 
have to cast shared away anyway, so atomic reading/writing wouldn't be 
applied there.


 I don't think there is an issue with converting unshared reference types
 to shared (ex. ref T -> ref shared(T) or T* -> shared(T)*); worst you
 get is some extra synchronization.


shared(T)* is not shared itself, so that's fine, yeah.

I think I've approached this from the wrong angle. Implicit conversions 
are not the problem. Reading/writing is. Copying a shared value to 
another shared location is unsafe, too, if not done atomically.
Jul 10 2016
next sibling parent ag0aep6g <anonymous example.com> writes:
On 07/11/2016 07:26 AM, ag0aep6g wrote:

 On 07/11/2016 01:52 AM, Alex Parrill wrote:


[...]

 I don't think there is an issue with converting unshared reference types
 to shared (ex. ref T -> ref shared(T) or T* -> shared(T)*); worst you
 get is some extra synchronization.


 shared(T)* is not shared itself, so that's fine, yeah.


The conversion T* -> shared(T)* isn't ok, of course. And it's correctly 
disallowed already. But it's for a different reason than this thread is 
about.
Jul 10 2016
prev sibling parent reply Kagamin <spam here.lot> writes:
On Monday, 11 July 2016 at 05:26:42 UTC, ag0aep6g wrote:

 Simply disallow reading and writing shared then, forcing 
 something more explicit like atomicLoad/atomicStore?

 That would be better than the current state, but it would make 
 shared even more unwieldy.


Atomic loads are only needed for volatile variables, not for all 
kinds of shared data. Also currently atomicLoad doesn't provide 
functionality equivalent to raw load.


 Generating atomic operations would break less code, and feels 
 like the obvious thing to me.


Multithreaded code can't be generated.
Jul 11 2016
parent reply ag0aep6g <anonymous example.com> writes:
On 07/11/2016 03:31 PM, Kagamin wrote:

 Atomic loads are only needed for volatile variables, not for all kinds
 of shared data.


Volatile just means that another thread can mess with the data, right? 
So shared data that's not being written to from elsewhere isn't 
volatile, and one doesn't need an atomic load to read it.

If I got that right: Sure. But the compiler can't know if a shared 
variable is volatile or not, so it has to assume that it is. If the 
programmer knows that it's not volatile, they can cast shared away and 
use a normal load.


 Also currently atomicLoad doesn't provide functionality
 equivalent to raw load.


Is a "raw load" just a non-atomic load, or is it something special?
What's the relevance of atomicLoad's capabilities?


 Generating atomic operations would break less code, and feels like the
 obvious thing to me.


 Multithreaded code can't be generated.


For primitive types, atomic loads and stores can be generated, no? It's 
clear that this doesn't make the code automatically thread-safe. It just 
guards against an easily made mistake. Like shared is supposed to do, as 
far as I understand.
Jul 11 2016
next sibling parent reply Kagamin <spam here.lot> writes:
On Monday, 11 July 2016 at 13:54:49 UTC, ag0aep6g wrote:

 If I got that right: Sure. But the compiler can't know if a 
 shared variable is volatile or not, so it has to assume that it 
 is. If the programmer knows that it's not volatile, they can 
 cast shared away and use a normal load.


If you cast shared away, an unshared postblit will be called 
instead of shared one.
Jul 12 2016
parent ag0aep6g <anonymous example.com> writes:
On 07/12/2016 02:26 PM, Kagamin wrote:

 On Monday, 11 July 2016 at 13:54:49 UTC, ag0aep6g wrote:

 If I got that right: Sure. But the compiler can't know if a shared
 variable is volatile or not, so it has to assume that it is. If the
 programmer knows that it's not volatile, they can cast shared away and
 use a normal load.


 If you cast shared away, an unshared postblit will be called instead of
 shared one.


True. One would have to watch out for that, and maybe work around 
correspondingly. The cast would be a greppable indicator that special 
care is needed. In contrast, unsafe reading/writing of shared data that 
looks like just another assignment is similarly problematic and harder 
to spot.

If reading/writing shared directly would be disallowed completely 
(always have to use core.atomic or cast), then shared postblits wouldn't 
make sense anymore, I guess.
Jul 12 2016
prev sibling parent reply Kagamin <spam here.lot> writes:
On Monday, 11 July 2016 at 13:54:49 UTC, ag0aep6g wrote:

 Also currently atomicLoad doesn't provide functionality
 equivalent to raw load.


 Is a "raw load" just a non-atomic load, or is it something 
 special?
 What's the relevance of atomicLoad's capabilities?


You suggested to use atomicLoad to access shared data. Raw atomic 
load is an assembler instruction and can't be optimized.
Jul 12 2016
parent reply ag0aep6g <anonymous example.com> writes:
On 07/12/2016 02:28 PM, Kagamin wrote:

 On Monday, 11 July 2016 at 13:54:49 UTC, ag0aep6g wrote:

 Also currently atomicLoad doesn't provide functionality
 equivalent to raw load.


 Is a "raw load" just a non-atomic load, or is it something special?
 What's the relevance of atomicLoad's capabilities?


 You suggested to use atomicLoad to access shared data. Raw atomic load
 is an assembler instruction and can't be optimized.


Correct me if I'm wrong: Currently, the compiler generates a non-atomic 
load for reading shared data. In particular, it doesn't generate a raw 
atomic load. And you say you can't do a raw atomic load with 
core.atomic.atomicLoad.

Then how does one do a raw atomic load at the moment? Does it become 
harder or impossible when the language is changed in one of the two 
suggested ways (generate another kind of atomic load / outright reject 
reading of shared types)?
Jul 12 2016
parent reply Kagamin <spam here.lot> writes:
On Tuesday, 12 July 2016 at 12:47:05 UTC, ag0aep6g wrote:

 In contrast, unsafe reading/writing of shared data that looks 
 like just another assignment is similarly problematic and 
 harder to spot.


It's strange if you want to expose a bare volatile variable to 
the rest of the world and expect the world to cope with 
concurrency on every access.


 If reading/writing shared directly would be disallowed 
 completely (always have to use core.atomic or cast), then 
 shared postblits wouldn't make sense anymore, I guess.


The problem is when an unshared postblit is called on the shared 
data.


 Correct me if I'm wrong: Currently, the compiler generates a 
 non-atomic load for reading shared data. In particular, it 
 doesn't generate a raw atomic load. And you say you can't do a 
 raw atomic load with core.atomic.atomicLoad.


I say non-atomic load is not supported by atomicLoad. 5th kind of 
synchronization will help here.
Jul 12 2016
parent reply ag0aep6g <anonymous example.com> writes:
On 07/12/2016 03:51 PM, Kagamin wrote:

 It's strange if you want to expose a bare volatile variable to the rest
 of the world and expect the world to cope with concurrency on every access.


I'm not sure what you're saying here. Should unsafe reading and writing 
of shared types simply be allowed and have the common syntax (as it is 
now), and the programmer should make sure that things are set up correctly?

That's a reasonable stance, but it's not the one that D takes towards 
shared when `shared int x; ++x` is deprecated.


 The problem is when an unshared postblit is called on the shared data.


I understand that. The programmer who writes the cast would have to work 
around it. I agree that it's rather ugly. Maybe a function can be 
written that does an unsafe copy and then calls the shared postblit.


 Correct me if I'm wrong: Currently, the compiler generates a
 non-atomic load for reading shared data. In particular, it doesn't
 generate a raw atomic load. And you say you can't do a raw atomic load
 with core.atomic.atomicLoad.


 I say non-atomic load is not supported by atomicLoad. 5th kind of
 synchronization will help here.


So a "raw load" is just a non-atomic load after all? When I asked 
earlier you wrote "raw atomic load" in your reply, so I assumed it was 
some kind of atomic load.

Supporting non-atomic load in atomicLoad would be weird, wouldn't it? It 
would be opposite of what it says on the label.

Would the point be that it calls the shared postblit (unlike casting 
shared away and then copying)? As mentioned above, I'd hope that a 
function that does that can be written.

I didn't manage to find out what the "5th kind of synchronization" is. 
If it's important for this discussion, please elaborate.
Jul 12 2016
parent reply Kagamin <spam here.lot> writes:
On Tuesday, 12 July 2016 at 14:38:27 UTC, ag0aep6g wrote:

 I'm not sure what you're saying here. Should unsafe reading and 
 writing of shared types simply be allowed and have the common 
 syntax (as it is now), and the programmer should make sure that 
 things are set up correctly?

 That's a reasonable stance, but it's not the one that D takes 
 towards shared when `shared int x; ++x` is deprecated.


shared only differentiates between shared and unshared data. 
Teaching people to write legit concurrent code is a different 
task. Increment of a shared variable doesn't have a compelling 
use case so whatever happens to it is not important, just storing 
shared data is more common.


 Supporting non-atomic load in atomicLoad would be weird, 
 wouldn't it? It would be opposite of what it says on the label.


The name can be different, e.g. fast load, which better reflects 
its purpose.


 Would the point be that it calls the shared postblit (unlike 
 casting shared away and then copying)? As mentioned above, I'd 
 hope that a function that does that can be written.


Postblit is a problem with casting. Performance is a problem with 
atomicLoad.
Jul 12 2016
parent reply ag0aep6g <anonymous example.com> writes:
On Tuesday, 12 July 2016 at 15:46:52 UTC, Kagamin wrote:

 shared only differentiates between shared and unshared data. 
 Teaching people to write legit concurrent code is a different 
 task. Increment of a shared variable doesn't have a compelling 
 use case so whatever happens to it is not important, just 
 storing shared data is more common.


The deprecation of ++ shows that (currently) shared's purpose is 
not only to differentiate between shared and unshared data.

Either we go forward with the deprecation of ++. Then reads and 
writes should follow the same pattern. Or we should revert the 
deprecation of ++ and say that people must simply be careful with 
shared.

The current inconsistency is no good.
Jul 12 2016
parent Kagamin <spam here.lot> writes:
On Tuesday, 12 July 2016 at 16:14:58 UTC, ag0aep6g wrote:

 The deprecation of ++ shows that (currently) shared's purpose 
 is not only to differentiate between shared and unshared data.


Feel free to submit a corresponding spec change PR stating that 
you want to extend shared's purpose to solve as many concurrency 
problems as possible at any cost and teach people to write 
concurrent code and have Walter review it.


 The current inconsistency is no good.


What we need is a shared FAQ with answers to common 
misunderstandings about it.
Jul 13 2016
prev sibling parent reply Steven Schveighoffer <schveiguy yahoo.com> writes:
On 7/10/16 9:02 AM, ag0aep6g wrote:

 While messing with atomicLoad [1], I noticed that dmd lets me implicitly
 convert values to/from shared without restrictions. It's in the spec
 [2]. This seems bad to me.


I think you misunderstand the problem here. Conversion means changing 
the type.

Once you have loaded the shared data into a register, or whatever, it's 
no longer shared, it's local. Writing it out to another place doesn't 
change anything. It's once you add references into the mix where you may 
have a problem.

What I think you mean (and I think you realize this now), is that the 
actual copying of the data should not be implicitly allowed. The type 
change is fine, it's the physical reading or writing of shared data that 
can cause issues. I agree we should extend the rules to prevent this.

In other words:

shared int x;

void main()
{
    // ++x; // not allowed
    int x2 = x + 1; // but this is
    x = x2; // and it shouldn't be
}

-Steve
Jul 11 2016
parent reply ag0aep6g <anonymous example.com> writes:
On 07/11/2016 02:54 PM, Steven Schveighoffer wrote:

 I think you misunderstand the problem here.


Yes.


 Conversion means changing
 the type.

 Once you have loaded the shared data into a register, or whatever, it's
 no longer shared, it's local. Writing it out to another place doesn't
 change anything. It's once you add references into the mix where you may
 have a problem.


Right.


 What I think you mean (and I think you realize this now), is that the
 actual copying of the data should not be implicitly allowed. The type
 change is fine, it's the physical reading or writing of shared data that
 can cause issues. I agree we should extend the rules to prevent this.


Exactly.


 In other words:

 shared int x;

 void main()
 {
     // ++x; // not allowed
     int x2 = x + 1; // but this is
     x = x2; // and it shouldn't be
 }


I think I would prefer if the compiler would generate atomic operations, 
but I'm clearly far from being an expert on any of this. Simply 
rejecting the code would be fine with me, too.

Also:

shared int x;
shared int y;
x = y; /* should be rejected too (or be atomic, if that's even possible) */
Jul 11 2016
parent ag0aep6g <anonymous example.com> writes:
On 07/11/2016 03:23 PM, ag0aep6g wrote:

 I think I would prefer if the compiler would generate atomic operations,


Backpedaling on that one.

With automatic atomic loads and stores, one could accidentally write this:

     shared int x;
     x = x + 1; /* atomic load + atomic != atomic increment */

Easy to miss the problem, because the code looks so innocent.

But when the atomic loads and stores must be spelled out it would look 
like in my original post:

     shared int x;
     atomicStore(x, atomicLoad(x) + 1);

Way more obvious that the code isn't actually thread-safe.

So now I'm leaning towards requiring the verbose version.
Jul 11 2016