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

--- Comment #10 from Walter Bright <bugzilla@digitalmars.com> ---
I have serious misgivings about this being even a bug. The monitor is outside of the type system, and should work regardless of the type of the class object. Also, I don't see a good reason why it shouldn't work for pure functions, too. So I may withdraw the PR to 'fix' it.

--

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

--- Comment #11 from ZombineDev <petar.p.kirov@gmail.com> ---
A const reference to an object may actually refer to a immutable object. If such object sits in ROM than failure of the compiler to prevent such error (modification of the monitor) may have bad consequences (depending on the implementation). Also most of the time, an object is being synchronized to ensure exclusive write access to it. However there's no point in that if the object is not mutable.

As for pure, personally I don't think that pure functions should be allowed to cause a deadlock (I consider it to be a global side effect). Furthermore, 99% of the time synchronized is used on shared objects, which pure functions have no business looking at.

--

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

--- Comment #12 from ZombineDev <petar.p.kirov@gmail.com> ---
(In reply to Walter Bright from comment #8)
> (In reply to ZombineDev from comment #7)
> > The OP issue doesn't mention class monitors. The bug also affects raw sync primitives like core.sync.mutex and in general everything using Object.IMonitor.
> 
> 'Klass' is a class, and so has a class monitor.

No, by OP, I meant Martin's comment.

E.g. https://github.com/dlang/druntime/blob/v2.071.2-b2/src/core/sync/mutex.d#L260 .

--

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

Andrei Alexandrescu <andrei@erdani.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |andrei@erdani.com

--- Comment #13 from Andrei Alexandrescu <andrei@erdani.com> ---
Can someone produce an example in which invariants promised by D's system are broken?

--

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

--- Comment #14 from Lodovico Giaretta <lodovico@giaretart.net> ---
(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.

--

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

--- Comment #15 from Andrei Alexandrescu <andrei@erdani.com> ---
(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.

--

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

--- Comment #16 from ZombineDev <petar.p.kirov@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
    }
}

--

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

--- Comment #17 from Martin Nowak <code@dawg.eu> ---
(In reply to Andrei Alexandrescu from comment #15)
> That's not the case. The compiler knows the object has mutable metadata and won't allow placing it in read-only pages.

Not allowing to put any class into read-only segments, just b/c someone might
want to synchronize on it, is not very convincing.
Also remember that we wanted to get rid of the extra 8-byte for monitor unless
explicitly requested
http://forum.dlang.org/post/xpliectmvwrwthamquke@forum.dlang.org.

Turning synchronized into a lowering for lock/unlock (with monitor support as fallback) would not only allow correct attribute inference (w/o making global decisions for everyone, @nogc, ...), it's also a less overhead for the core.sync classes to not go through the virtual monitor indirections [¹].

[¹]: https://github.com/dlang/druntime/blob/15a227477a344583c4748d95492703901417f4f8/src/rt/monitor_.d#L59

--

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

--- Comment #18 from Martin Nowak <code@dawg.eu> ---
(In reply to Andrei Alexandrescu from comment #13)
> Can someone produce an example in which invariants promised by D's system are broken?

Just look at core.sync, none of the methods can be implemented const or pure,
still they get called from const/pure code.
In fact Object.Monitor simply declares that lock/unlock doesn't need to have
any attributes
https://github.com/dlang/druntime/blob/e9c7878928330aa34e6ba5c5863ed5507e02248e/src/object.d#L97-L101,
but synchronized forges guarantees that aren't there.

--

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

--- Comment #19 from Andrei Alexandrescu <andrei@erdani.com> ---
(In reply to Martin Nowak from comment #18)
> (In reply to Andrei Alexandrescu from comment #13)
> > Can someone produce an example in which invariants promised by D's system are broken?
> 
> Just look at core.sync, none of the methods can be implemented const or pure, still they get called from const/pure code.

That's not a problem, the runtime is expected to contain nonportable code.

--