Issue 20484 - Regression: runtime cannot handle shared(AA)
Summary: Regression: runtime cannot handle shared(AA)
Status: NEW
Alias: None
Product: D
Classification: Unclassified
Component: druntime (show other issues)
Version: D2
Hardware: x86_64 Mac OS X
: P1 regression
Assignee: No Owner
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-01-06 13:14 UTC by James Blachly
Modified: 2024-12-07 13:39 UTC (History)
1 user (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this issue.
Description James Blachly 2020-01-06 13:14:24 UTC
I posted question related to the following on D.learn on 2019/11/17.
https://forum.dlang.org/thread/qqrpu5$1nft$1@digitalmars.com


Summary:
--------
We experience failures to compile codebase with shared(AA) beginning with frontend version 2.087.

Culprit:
--------
Commit/lines https://github.com/dlang/druntime/commit/af86e7db58f4b792e45bdc3ee43c17aadc9d54ab#diff-a68e58fcf0de5aa198fcaceafe4e8cf9L3430-R3454 seems to have introduced a regression whereby the runtime cannot handle shared(AA). I should note that this commit ("object: Declare private struct AA and fix aaA function signatures ") is not mentioned in the changelog which made tracking it down much harder. 

Example:
--------
A minimal reproducible example is available here: https://gist.github.com/jblachly/78c5762bbfea65b09e7a1417ad763019 (shorten function on run.dlang.io is still broken) and is pasted below. You can select "all DMD compilers" on run.dlang.io to see that this breaks at 2.087.1.


```
import std.stdio;

struct B
{
    int[int] xx;
}

struct A
{
    B b;
}

void main()
{
    shared A a;
    
    a.b.xx[1] = 2;
    a.b.xx[2] = 4;
    a.b.xx[4] = 8;
    
    foreach(int k; a.b.xx.keys)
        writeln(k);
    foreach(int v; a.b.xx.values)
        writeln(v);
}
```

Output:
```
           2.087.1: Failure with output:
-----
/path/to/dmd.linux/dmd2/linux/bin64/../../src/druntime/import/object.d(3417): Error: cannot implicitly convert expression `aa` of type `shared(int[int])` to `const(shared(int)[int])`
onlineapp.d(21): Error: template instance `object.keys!(shared(int[int]), shared(int), int)` error instantiating
/path/to/dmd.linux/dmd2/linux/bin64/../../src/druntime/import/object.d(3468): Error: cannot implicitly convert expression `aa` of type `shared(int[int])` to `const(shared(int)[int])`
onlineapp.d(23): Error: template instance `object.values!(shared(int[int]), shared(int), int)` error instantiating
```

Thanks all for your help
Comment 1 ZombineDev 2020-02-18 21:54:29 UTC
Shared AAs were never properly supported in druntime. I would say that before the compiler was accepting invalid code. The reason is that the AA implementation does not implement internal synchronization, and instead assumes that all AA instances are thread-local.

The (pedantically) correct way to go is:
1. Keep using the `shared` type qualifier. It will prevent accidental mutation of shared of non-internally synchronized data.
2. Use some form of external synchronization (mutex, read-write lock, etc.) to ensure that there can be either a single writer or zero or more readers.
3. In a block of code where you have acquired exclusive write access (by means of external synchronization), it's ok to cast the AA from type `shared(K[V])` to `shared(K)[V]` or simply `K[V]` (*).
In a block where you have acquired read-access, you can safely cast from `shared(K[V])` to `const(shared(K)[V])` or `const(K[V])` (*) 

(*) The distinction between shared(K)[V] and simply K[V] may be subtle, but it's important and needs to be decided by the author of the code on a case-by-case basis.

For example:

A) The type `int[][string]` allows mutation of both the AA (e.g. getting, adding and removing int[] arrays) and each value (e.g. resizing the arrays and changing each element).

B) With `shared(int[][string])` you can't do anything since druntime does not (yet) implement internal synchronization of the AA implementation.

C) With `shared(int)[][string]` you can get, add and remove values from the AA, and you can change the values by making them point to different arrays, but the array elements can't be read/written to (without e.g. atomicLoad / atomicStore / cas)

D) `shared(int[])[string]` is kind of degenerate case between B) and C), but pedantically speaking it can't work correctly, because to do so, the AA implementation needs to use atomic load/store in order to guarantee the safe mutation of values.
Comment 2 James Blachly 2020-02-18 22:16:48 UTC
@ZombineDev: Thanks for the details. Our codebase was already using synchronization.

The runtime is still making a type error that causes compilation failure, but I do not understand runtime internals well enough to appreciate whether the offending block can simply be rolled back.
Comment 3 dlangBugzillaToGithub 2024-12-07 13:39:52 UTC
THIS ISSUE HAS BEEN MOVED TO GITHUB

https://github.com/dlang/dmd/issues/17394

DO NOT COMMENT HERE ANYMORE, NOBODY WILL SEE IT, THIS ISSUE HAS BEEN MOVED TO GITHUB