Runtime: Guarantee that newly allocated memory can't be stale for any thread

[jnm2]

Is it possible for another thread to have a stale view of the memory containing the fields that a constructor just initialized?

I would guess for safety there would have to be a runtime guarantee of a write barrier every time an instance is allocated, or some other guarantee that there's no way the memory could be stale for other threads, but I'm not sure and I would like a definitive answer.

This is what it comes down to, practically. In this example, does this.action = action need to be Volatile.Write(ref this.action, action)?
Or does the runtime take care of this with 100% certainty via implicit write barrier or other means so that I can't possibly need Volatile.Write in the constructor (assuming I don't share this with another thread during construction, of course)?

public sealed class OnDisposeAction : IDisposable
{
    private Action action;

    // Thread 1 does this:
    public OnDisposeAction(Action action)
    {
        this.action = action;
    }

    // Interlocked because sometimes multiple threads race for this,
    // but for the purposes of this question, only thread 2 ever calls this:
    public void Dispose() => Interlocked.Exchange(ref action, null)?.Invoke();
}

// Thread 1
Volatile.Write(sharedObject._x, new OnDisposeAction(() => { }));
// sharedObject._x.action is not null
// (waits)

// Thread 2
Volatile.Read(ref sharedObject._x).Dispose();
// sharedObject._x.action is disposed and set to null; the write barrier
// causes shared memory to be updated to `null` at the address of sharedObject._x.action
// (waits)

// Thread 1
Volatile.Write(sharedObject._x, null);
GC.Collect();
// Let's suppose the runtime happens to pick the exact same memory location
// to allocate the new OnDisposeAction that it used for the now-freed previous instance:
Volatile.Write(sharedObject._x, new OnDisposeAction(() => { }));
// If the constructor does not guarantee an implicit write barrier,
// shared memory still has `null` at the address of sharedObject._x.action.
// (waits)

// Thread 2
Volatile.Read(ref sharedObject._x).Dispose();
// This should have disposed the instance, but instead it no-ops.
// Interlocked.Exchange synced thread 2's memory with shared memory
// but shared memory still has `null` for this memory address.
// Thread 1's memory hasn't pushed the write of `this.action = action` to shared memory.

Thank you.

/cc @maoni and @swgillespie (via @tannergooding)
/cc @sharwell from conversation

[jnm2]

@Drawaes informed me that Volatile.Write is for ordering and adds a read barrier, but all writes use a write barrier.

So staleness is definitively not a problem and we're only talking about reordering. Is there any way writes could be reordered so that a second thread could observe the new instance reference once the constructor returns, and dereference it to read the field and observe a stale null because the writes were observed out of order?

[Drawaes]

On Intel CPU's :) and the as I understand it, ARM I am not sure about :)

[benaadams]

Let's suppose the runtime happens to pick the exact same memory location to allocate

GC would have also written to the location to zero it out?

Aside: Wouldn't it be easier to mark _x as volatile in the above?

May help? The C# Memory Model in Theory and Practice, Part 2

[jnm2]

GC would have also written to the location to zero it out?

Yes, and same ordering question applies.

Aside: Wouldn't it be easier to mark _x as volatile in the above?

If I'm reading http://joeduffyblog.com/2008/06/13/volatile-reads-and-writes-and-timeliness/ right, the volatile keyword causes only write fences on writes and so is a no-op because all writes are write-fenced anyway, while Volatile.Write causes a full fence for ordering guarantees.

[omariom]

I would rely on formal CLI specifications. Every compiler - C#, JIT or AOT - will adhere to them.

[benaadams]

I would rely on formal CLI specifications.

CLI spec says:

I.12.6.8 Other memory model issues

A conforming implementation of the CLI shall ensure that, even in a multi-threaded environment
and without proper user synchronization, objects are allocated in a manner that prevents
unauthorized memory access and prevents invalid operations from occurring. In particular, on
multiprocessor memory systems where explicit synchronization is required to ensure that all
relevant data structures are visible (for example, vtable pointers) the Execution Engine shall be
responsible for either enforcing this synchronization automatically or for converting errors due to
lack of synchronization into non-fatal, non-corrupting, user-visible exceptions.

It is explicitly not a requirement that a conforming implementation of the CLI guarantee that all
state updates performed within a constructor be uniformly visible before the constructor
completes. CIL generators can ensure this requirement themselves by inserting appropriate calls
to the memory barrier or volatile write instructions.

Different reason, same problem? Would suggest you need the Volatile.Write?

[jnm2]

Thanks Ben. That does sound pretty conclusive. Especially considering C# may run on multiple runtimes, I wouldn't want to depend on something that isn't a guarantee in all of them. We have CLR, CoreCLR, CoreRT, Mono, Unity, Blazor, etc.

And since I can see that the CIL generator (Roslyn) is not emitting a barrier or volatile write operation in IL, that means I do indeed need the Volatile.Write to run on any conforming VM.

The spec doesn't differentiate between state updates in the newly allocated memory and state updates external to the new instance, does it? Link handy?

[benaadams]

I looked at:
Standard ECMA-335 Common Language Infrastructure (CLI) 6th edition (June 2012)
http://www.ecma-international.org/publications/standards/Ecma-335.htm

[jkotas]

volatile keyword, Volatile.Write

Both of these, as well as volatile. IL prefix, work the same. There is no difference in the guarantees that they provide.

[benaadams]

private volatile Action action; :)

[jnm2]

@benaadams I can't make Action volatile or the compiler yells at me for using ref on it for Interlocked.Exchange.

(Interlocked.Exchange is necessary to guarantee that two threads racing to dispose the object can only invoke the action once.)

[benaadams]

There's a pragma for that ;)

[jnm2]

Plus to me it's counter-intuitive that you'd mark the storage location as volatile rather than the load/store operation. I wish the volatile keyword did nothing more or less than force you to access it only as a Volatile or Interlocked ref target.

[Drawaes]

Do what Duffy suggested. Write a struct wrapper where you can only access
or update the values via barriers;)

On 15/10/2017 16:05, "Joseph Musser" [e-mail] wrote:

Plus to me it's counter-intuitive that you'd mark he storage location as
volatile rather than the load/store operation. I wish the volatile
keyword did nothing more or less than force you to access it only as a
Volatile or Interlocked ref target.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/dotnet/coreclr/issues/14508#issuecomment-336717610,
or mute the thread
https://github.com/notifications/unsubscribe-auth/APpZuZKkktNKJP62poo1_jxriipUaW_fks5ssh8jgaJpZM4P5gfU
.

[omariom]

In this example, does this.action = action need to be Volatile.Write(ref this.action, action)?

This is not enough as Volatile.Write is a store-release barrier and such write can still be reordered with the subsequent normal store of the OnDisposeAction object.

[svick]

@jnm2

I can't make Action volatile or the compiler yells at me for using ref on it for Interlocked.Exchange.

It doesn't yell, the compiler does not generate that warning for Interlocked methods. You might assume it does, because it did in the past (before Roslyn).

[jnm2]

❤️ https://sharplab.io/#v2:EYLgtghgzgLgpgJwD4AEAMACFBGAdAFQAsE4IATASwDsBzAbgFgAoZlAZiwCYMBhZgb2YZhGAA4IKANwjwMkgPYAbGRUVwM1GBgAejJiIxCR7LABYMAWQAUASiPDB+gyICSVeAkXyAxgGs4ZLgAotrehBC0cFYkAGY6ADQY2DZ6BgC+zGlAA