[Issue 14251] synchronized (mtx) doesn't check attributes (pure, const)

[via Digitalmars-d-bugs]

https://issues.dlang.org/show_bug.cgi?id=14251

--- Comment #16 from ZombineDev <petar.p.kirov at gmail.com> ---

(In reply to Andrei Alexandrescu from comment #15)
> (In reply to Lodovico Giaretta from comment #14)
> > (In reply to Andrei Alexandrescu from comment #13)
> > > Can someone produce an example in which invariants promised by D's system
> > > are broken?
> > 
> > immutable provides a strong guarantee, that allows me to put my immutable
> > data in ROM. If I manage to have an immutable object allocated in ROM and
> > someone tries to synchronize on it, my program will (in the best case) crash
> > with a segmentation fault, as the synchronized statement tries to modify a
> > mutex that is located in ROM.
> 
> That's not the case. The compiler knows the object has mutable metadata and
> won't allow placing it in read-only pages.

Wrong. See for yourself: https://dpaste.dzfl.pl/be0f23bf35c0

BTW, what do you think about pure? Should locking of shared objects really be
allowed in pure code? According to
http://dlang.org/spec/function.html#pure-functions:
> a pure function: does not read or write any global or static mutable state

// Live demo: https://dpaste.dzfl.pl/be0f23bf35c0
import core.sync.mutex : Mutex;

void main()
{
    // case 1: Allows unsafe use of core.sync.Mutex
    const stdMutex = new const Mutex(); 
    constAndpurityTest(stdMutex);

    // case 2: Breaks immutability guarantee
    immutable myMutex = new immutable MyMutex();
    assert (myMutex.flag == 0);

    //myMutex.lock(); // correctly disalowed

    synchronized(myMutex)
    {
        // my fail depending on the optimization level
        assert (myMutex.flag == 1); // wrong!!!
    }

    // case 3: Modifies normal object that could be stored in ROM
    immutable c = new immutable C();
    assert (c.__monitor is null);

    synchronized (c) { }

    assert (c.__monitor !is null); // WRONG!
}

class C { }

class MyMutex : Object.Monitor
{
    int flag;

    // See
https://github.com/dlang/druntime/blob/v2.071.2-b2/src/rt/monitor_.d#L204
    // and
https://github.com/dlang/druntime/blob/v2.071.2-b2/src/core/sync/mutex.d#L81
    // for details.
    Object.Monitor necessaryIndirection;

    this() pure immutable
    {
        this.necessaryIndirection = this;
        this.__monitor = cast(void*)&this.necessaryIndirection;
    }

    @trusted void lock()
    {
        this.flag++;
    }

    @trusted void unlock()
    {
        //this.flag--;
    }
}

void constAndpurityTest(const Mutex stdMutex) pure
{
    import std.traits : FA = FunctionAttribute, fattrs = functionAttributes;

    auto stdMutexLock = &stdMutex.lock;

    static assert((fattrs!stdMutexLock & FA.pure_) == 0);
    static assert((fattrs!stdMutexLock & FA.const_) == 0);

    synchronized (stdMutex) // Accepts invalid!
    {
        // synchronized happily calls the core.sync.Mutex.lock() method which
is
        // neither pure, nor const
    }
}

--