Fixing tail mutable TLS/field initializers

Jun 07

Nick Treleaven

Jun 10

Nick Treleaven

Jun 10

Steven Schveighoffer

Jun 12

Dukc

June 07

Fixing tail mutable TLS/field initializers

Posted by Nick Treleaven

Permalink

Nick Treleaven

Permalink

int[] x = [1, 2, 3];

The above is really like:

__gshared gsx = [1, 2, 3];
int[] x = gsx;

Except that lowering is not valid D, because gsx 'cannot be read at compile time'.

So if one thread mutates x[0], even though x is thread-local, existing and new threads will get their x[0] contents mutated too (assuming x still points to the initializer data).

Note that any tail mutable initializer has the same problem, not just array literals.

It gets worse with an aggregate type T field initializer - a mutable instance can mutate the initializer contents of an immutable T instance field:
https://issues.dlang.org/show_bug.cgi?id=10376

The immutable violation problem must be fixed. There are 2 solutions for that proposed:

Timon (in 2013): Having two versions of the initializer, one for mutable field instances in the writable data segment and one for immutable field instances in the non-writable data segment. Possibly this solution could cause subtle issues, like not allowing a temporary unique mutable instance to be cast to immutable (e.g. pure factory functions).
Walter: Disallowing constructing an immutable(T) if it has fields pointing to mutable initializers. Instead, use cast(immutable) on a new instance, which is not allowed in @safe code. This could mean such library types not unit tested with immutable won't work with immutable in user code, that otherwise could do (bug-prone, but possible to use correctly).

Neither of these attempt to solve the problem of accidental mutation of a field when the programmer does not expect the initializer to be shared across instances. That is something that gets discovered repeatedly (e.g. yesterday). And has been filed as a bug many times (see duplicates and duplicates of duplicates!):
https://issues.dlang.org/show_bug.cgi?id=2947

Given that we will have editions, ideally we would disallow any tail mutable initializer for both fields and thread-local variables. They are bug-prone - use a constructor instead if a unique initializer is intended.

One reason why we might not want to disallow them is if it makes porting existing code to the next edition too awkward. So we would need to investigate that.

On Friday, 7 June 2024 at 11:05:03 UTC, Nick Treleaven wrote:

int[] x = [1, 2, 3];

The above is really like:

__gshared gsx = [1, 2, 3];
int[] x = gsx;

Rather:

__gshared int[3] gsx = [1, 2, 3];

So x is a TLS slice of __gshared data.

And given that __gshared mutable variables are not allowed to be accessed from @safe code (including basic data types), tail mutable initializers should also be disallowed in safe code, to prevent data races.

So if one thread mutates x[0], even though x is thread-local, existing and new threads will get their x[0] contents mutated too (assuming x still points to the initializer data).

Mutable shared globals & fields can still be allowed to use tail mutable initializers.

Nick Treleaven kirjoitti 7.6.2024 klo 14.05: > ```d > int[] x = [1, 2, 3]; > ``` > The above is really like: > ```d > __gshared gsx = [1, 2, 3]; > int[] x = gsx; > ``` > [snip] Unless we want to disallow array literals for mutable static variables, there's really only one solution: the array has to be duplicated at module initialisation time, any time a thread is initialised: ```d __gshared gsx = [1, 2, 3]; int[] x; static this() { x = gsx.dup; } ```

Forums