• Andrew Wiley (118/118) Dec 22 2011 I'm having some issues getting the synchronized modifier to work like
• Somedude (5/133) Dec 23 2011 On windows XP with DMD 2.057, I get
• Andrew Wiley (28/32) Dec 23 2011 Yes, I posted another (much shorter) post describing the issue with
• Andrew Wiley (37/64) Dec 23 2011 ote:
• Sean Kelly (3/12) Jan 04 2012 The ctor should really be shared.=
• Sean Kelly (7/47) Jan 04 2012 wrote:
• Andrew Wiley (12/54) Jan 04 2012 wrote:
• Sean Kelly (33/41) Jan 04 2012 best you'll get a memory leak here.
• Steven Schveighoffer (11/52) Jan 07 2012 Can Mutex do this?
• Sean Kelly (21/63) Jan 07 2012 There's still a race condition between unlocking the monitor and freeing...
• Steven Schveighoffer (5/7) Jan 07 2012 Wouldn't that result in a segfault or other error? How is that worse th...
• Sean Kelly (9/16) Jan 07 2012 Right now, Mutex will assert if you assign it to an object with a monito...
• Steven Schveighoffer (4/6) Jan 09 2012 In other words, it doesn't :)
• Sean Kelly (7/14) Jan 09 2012 Which goes back to my argument that we should ship both checked and unch...
• Steven Schveighoffer (9/12) Jan 09 2012 Or you make it a release-mode check. I can't imagine this being an
• Sean Kelly (12/21) Jan 09 2012 I know. But once I add the ability to replace a mutex there's no way to ...
• Andrew Wiley (5/7) Jan 09 2012 I agree. This is common in the C/C++ world, and it seems even more
• Andrew Wiley (16/46) Jan 04 2012 st you'll get a memory leak here.
• Sean Kelly (15/53) Jan 05 2012 It's just like ctors of invariant classes being invariant. Initializatio...

Andrew Wiley <wiley.andrew.j gmail.com> writes:

I'm having some issues getting the synchronized modifier to work like
a synchronized block. Sorry this code example is long, but here's
three dummy implementations of a Condition-based work queue:

---import std.stdio;
import core.sync.mutex;
import core.sync.condition;
import core.thread;

synchronized class Queue1 {
private:
    bool _work;
    Condition _cond;
public:
    this() {
        auto lock = new Mutex(this); // initialize the monitor for
this object so we can use the same lock in the Condition
        _cond = new Condition(lock);
        _work = false;
    }
    void doWork() {
        while(!_work) (cast()_cond).wait();
        _work = false;
        writeln("did work");
        return;
    }
    void addWork() {
        _work = true;
        (cast()_cond).notify();
        writeln("added work");
    }
}

class Queue2 {
private:
    bool _work;
    Condition _cond;
public:
    this() {
        auto lock = new Mutex(this); // again, initialize the monitor
explicitly so we can reuse the lock
        _cond = new Condition(lock);
        _work = false;
    }
    synchronized void doWork() {
        while(!_work) (cast()_cond).wait();
        _work = false;
        writeln("did work");
        return;
    }
    synchronized void addWork() {
        _work = true;
        (cast()_cond).notify();
        writeln("added work");
    }
}

class Queue3 {
private:
    bool _work;
    Condition _cond;
    Mutex _lock;
public:
    this() {
        _lock = new Mutex(this); // make our own lock and ignore the monitor
        _cond = new Condition(_lock);
        _work = false;
    }
    void doWork() shared {
        synchronized(_lock) {
            while(!_work) (cast()_cond).wait();
            _work = false;
            writeln("did work");
            return;
        }
    }
    void addWork() shared {
        synchronized(_lock) {
            _work = true;
            (cast()_cond).notify();
            writeln("added work");
       }
    }
}

void main() {
    version(Q1) auto queue = new shared(Queue1)();
    version(Q2) auto queue = new shared(Queue2)();
    version(Q3) auto queue = new shared(Queue3)();
    auto thread = new Thread({
        while(true) queue.doWork();
        });
    thread.start();
    while(true) queue.addWork();
}
---

As you can see, this program starts a producer thread and a consumer
thread, and it should endlessly print "did work" and "added work".
Because the consumer has to wake up from the condition variable
periodically, the producer tends to print far more often.

So first, according to the D spec, I believe these three queues should
behave identically. Is that true?

Here's what happens:
Queue1:
  DMD: crash
  GDC: crash
    Both print errors that look like this:
core.sync.exception.SyncException src/core/sync/mutex.d(159): Unable
to unlock mutex
    From GDC's stack trace, it looks like the constructor call is
being synchronized, so it crashes when I swap out the lock halfway
through the constructor. Do we really need to synchronize constructor
calls? I've never heard of another language that does this.

Queue2:
  DMD: works
  GDC: deadlock
    Unless someone can see an issue here, I guess this is a GDC bug
and should be handled separately.

Queue3:
  DMD: works
  GDC: works
    This is about the most problematic way to do locking, but it does
seem to work reliably.

Somedude <lovelydear mailmetrash.com> writes:

Le 22/12/2011 10:47, Andrew Wiley a écrit :
 I'm having some issues getting the synchronized modifier to work like
 a synchronized block. Sorry this code example is long, but here's
 three dummy implementations of a Condition-based work queue:
 
 ---import std.stdio;
 import core.sync.mutex;
 import core.sync.condition;
 import core.thread;
 
 synchronized class Queue1 {
 private:
     bool _work;
     Condition _cond;
 public:
     this() {
         auto lock = new Mutex(this); // initialize the monitor for
 this object so we can use the same lock in the Condition
         _cond = new Condition(lock);
         _work = false;
     }
     void doWork() {
         while(!_work) (cast()_cond).wait();
         _work = false;
         writeln("did work");
         return;
     }
     void addWork() {
         _work = true;
         (cast()_cond).notify();
         writeln("added work");
     }
 }
 
 class Queue2 {
 private:
     bool _work;
     Condition _cond;
 public:
     this() {
         auto lock = new Mutex(this); // again, initialize the monitor
 explicitly so we can reuse the lock
         _cond = new Condition(lock);
         _work = false;
     }
     synchronized void doWork() {
         while(!_work) (cast()_cond).wait();
         _work = false;
         writeln("did work");
         return;
     }
     synchronized void addWork() {
         _work = true;
         (cast()_cond).notify();
         writeln("added work");
     }
 }
 
 class Queue3 {
 private:
     bool _work;
     Condition _cond;
     Mutex _lock;
 public:
     this() {
         _lock = new Mutex(this); // make our own lock and ignore the monitor
         _cond = new Condition(_lock);
         _work = false;
     }
     void doWork() shared {
         synchronized(_lock) {
             while(!_work) (cast()_cond).wait();
             _work = false;
             writeln("did work");
             return;
         }
     }
     void addWork() shared {
         synchronized(_lock) {
             _work = true;
             (cast()_cond).notify();
             writeln("added work");
        }
     }
 }
 
 void main() {
     version(Q1) auto queue = new shared(Queue1)();
     version(Q2) auto queue = new shared(Queue2)();
     version(Q3) auto queue = new shared(Queue3)();
     auto thread = new Thread({
         while(true) queue.doWork();
         });
     thread.start();
     while(true) queue.addWork();
 }
 ---
 
 As you can see, this program starts a producer thread and a consumer
 thread, and it should endlessly print "did work" and "added work".
 Because the consumer has to wake up from the condition variable
 periodically, the producer tends to print far more often.
 
 So first, according to the D spec, I believe these three queues should
 behave identically. Is that true?
 
 Here's what happens:
 Queue1:
   DMD: crash
   GDC: crash
     Both print errors that look like this:
 core.sync.exception.SyncException src/core/sync/mutex.d(159): Unable
 to unlock mutex
     From GDC's stack trace, it looks like the constructor call is
 being synchronized, so it crashes when I swap out the lock halfway
 through the constructor. Do we really need to synchronize constructor
 calls? I've never heard of another language that does this.
 
 Queue2:
   DMD: works
   GDC: deadlock
     Unless someone can see an issue here, I guess this is a GDC bug
 and should be handled separately.
 
 Queue3:
   DMD: works
   GDC: works
     This is about the most problematic way to do locking, but it does
 seem to work reliably.

On windows XP with DMD 2.057, I get
Queue1: deadlock
Queue2: works
Queue3: works

Andrew Wiley <wiley.andrew.j gmail.com> writes:

On Fri, Dec 23, 2011 at 1:25 AM, Somedude <lovelydear mailmetrash.com> wrot=
e:
 On windows XP with DMD 2.057, I get
 Queue1: deadlock
 Queue2: works
 Queue3: works

Yes, I posted another (much shorter) post describing the issue with
Queue1. In short, since Queue1 is a synchronized class, the
constructor is synchronized (which is mostly worthless). As a
consequence, when I replace the lock in the middle of the function,
bad things happen when the runtime tries to unlock the lock at the end
and sees the new lock.

This version of Queue1 shows a very hacky way to get around this:
---
synchronized class Queue1 {private:=A0 =A0bool _work;=A0 =A0Condition
_cond;public:=A0 =A0this() {=A0 =A0 =A0 =A0auto lock =3D new Mutex(this); /=
/
initialize the monitor for this object so we can use the same lock in
the Condition       lock.lock(); // HACK: acquire the lock so we can
unlock it at the end of the function=A0 =A0 =A0 =A0_cond =3D new
Condition(lock);=A0 =A0 =A0 =A0_work =3D false;=A0 =A0}=A0 =A0void doWork()=
 {
while(!_work) (cast()_cond).wait();=A0 =A0 =A0 =A0_work =3D false;
writeln("did work");=A0 =A0 =A0 =A0return;=A0 =A0}=A0 =A0void addWork() {=
=A0 =A0 =A0 =A0_work
=3D true;=A0 =A0 =A0 =A0(cast()_cond).notify();=A0 =A0 =A0 =A0writeln("adde=
d work");
}}---
Queue2 looks like a bug where GDC is acquiring all locks twice in
synchronized functions, and since the condition variable only unlocks
the lock once, a deadlock results. I'll get a bug report up about it
shortly.

Andrew Wiley <wiley.andrew.j gmail.com> writes:

On Fri, Dec 23, 2011 at 8:33 PM, Andrew Wiley <wiley.andrew.j gmail.com> wr=
ote:
 On Fri, Dec 23, 2011 at 1:25 AM, Somedude <lovelydear mailmetrash.com> wr=

ote:
 On windows XP with DMD 2.057, I get
 Queue1: deadlock
 Queue2: works
 Queue3: works

 Yes, I posted another (much shorter) post describing the issue with
 Queue1. In short, since Queue1 is a synchronized class, the
 constructor is synchronized (which is mostly worthless). As a
 consequence, when I replace the lock in the middle of the function,
 bad things happen when the runtime tries to unlock the lock at the end
 and sees the new lock.

 This version of Queue1 shows a very hacky way to get around this:
 ---
 synchronized class Queue1 {private:=A0 =A0bool _work;=A0 =A0Condition
 _cond;public:=A0 =A0this() {=A0 =A0 =A0 =A0auto lock =3D new Mutex(this);=

 //
 initialize the monitor for this object so we can use the same lock in
 the Condition =A0 =A0 =A0 lock.lock(); // HACK: acquire the lock so we ca=

n
 unlock it at the end of the function=A0 =A0 =A0 =A0_cond =3D new
 Condition(lock);=A0 =A0 =A0 =A0_work =3D false;=A0 =A0}=A0 =A0void doWork=

() {
 while(!_work) (cast()_cond).wait();=A0 =A0 =A0 =A0_work =3D false;
 writeln("did work");=A0 =A0 =A0 =A0return;=A0 =A0}=A0 =A0void addWork() {=

=A0 =A0 =A0 =A0_work
 =3D true;=A0 =A0 =A0 =A0(cast()_cond).notify();=A0 =A0 =A0 =A0writeln("ad=

ded work");
 }}---
 Queue2 looks like a bug where GDC is acquiring all locks twice in
 synchronized functions, and since the condition variable only unlocks
 the lock once, a deadlock results. I'll get a bug report up about it
 shortly.

Gah, my hacked/fixed Queue1 got garbled:
---
synchronized class Queue1 {
private:
 =A0 =A0bool _work;
 =A0 =A0Condition _cond;
public:
 =A0 =A0this() {
=A0 =A0 =A0 =A0 auto lock =3D new Mutex(this); // initialize the monitor fo=
r this object
 =A0 =A0 =A0 =A0// so we can use the same lock in the Condition
 =A0 =A0 =A0  lock.lock(); // HACK: acquire the lock so we can unlock it
 =A0 =A0 =A0 =A0// at the end of the function
 =A0 =A0 =A0 =A0_cond =3D new Condition(lock);
 =A0 =A0 =A0 =A0_work =3D false;
 =A0 =A0}
 =A0 =A0void doWork() {
 =A0 =A0 =A0 =A0while(!_work) (cast()_cond).wait();
 =A0 =A0 =A0 =A0_work =3D false;
 =A0 =A0 =A0 =A0writeln("did work");
 =A0 =A0 =A0 =A0return;
 =A0 =A0}
 =A0 =A0void addWork() {
 =A0 =A0 =A0 =A0_work =3D true;
 =A0 =A0 =A0 =A0(cast()_cond).notify();
 =A0 =A0 =A0 =A0writeln("added work");
 =A0 =A0}
}
---

Sean Kelly <sean invisibleduck.org> writes:

On Dec 23, 2011, at 8:33 PM, Andrew Wiley wrote:

 On Fri, Dec 23, 2011 at 1:25 AM, Somedude <lovelydear mailmetrash.com> =

wrote:
 On windows XP with DMD 2.057, I get
 Queue1: deadlock
 Queue2: works
 Queue3: works

=20
 Yes, I posted another (much shorter) post describing the issue with
 Queue1. In short, since Queue1 is a synchronized class, the
 constructor is synchronized (which is mostly worthless).

The ctor should really be shared.=

Sean Kelly <sean invisibleduck.org> writes:

On Dec 23, 2011, at 8:49 PM, Andrew Wiley wrote:

 On Fri, Dec 23, 2011 at 8:33 PM, Andrew Wiley =

<wiley.andrew.j gmail.com> wrote:
 On Fri, Dec 23, 2011 at 1:25 AM, Somedude =


<lovelydear mailmetrash.com> wrote:
 On windows XP with DMD 2.057, I get
 Queue1: deadlock
 Queue2: works
 Queue3: works

=20
 Yes, I posted another (much shorter) post describing the issue with
 Queue1. In short, since Queue1 is a synchronized class, the
 constructor is synchronized (which is mostly worthless). As a
 consequence, when I replace the lock in the middle of the function,
 bad things happen when the runtime tries to unlock the lock at the =


end
 and sees the new lock.
=20
 This version of Queue1 shows a very hacky way to get around this:
 ---
 synchronized class Queue1 {private:   bool _work;   Condition
 _cond;public:   this() {       auto lock =3D new Mutex(this); //
 initialize the monitor for this object so we can use the same lock in
 the Condition       lock.lock(); // HACK: acquire the lock so we can
 unlock it at the end of the function       _cond =3D new
 Condition(lock);       _work =3D false;   }   void doWork() {
 while(!_work) (cast()_cond).wait();       _work =3D false;
 writeln("did work");       return;   }   void addWork() {       _work
 =3D true;       (cast()_cond).notify();       writeln("added work");
 }}---
 Queue2 looks like a bug where GDC is acquiring all locks twice in
 synchronized functions, and since the condition variable only unlocks
 the lock once, a deadlock results. I'll get a bug report up about it
 shortly.

=20
 Gah, my hacked/fixed Queue1 got garbled:
 ---
 synchronized class Queue1 {
 private:
    bool _work;
    Condition _cond;
 public:
    this() {
         auto lock =3D new Mutex(this); // initialize the monitor for =

this object

This assumes that there is no existing monitor for the object.  At best =
you'll get a memory leak here.=

Andrew Wiley <wiley.andrew.j gmail.com> writes:

On Wed, Jan 4, 2012 at 2:12 PM, Sean Kelly <sean invisibleduck.org> wrote:
 On Dec 23, 2011, at 8:49 PM, Andrew Wiley wrote:

 On Fri, Dec 23, 2011 at 8:33 PM, Andrew Wiley <wiley.andrew.j gmail.com>=


 wrote:
 On Fri, Dec 23, 2011 at 1:25 AM, Somedude <lovelydear mailmetrash.com> =



wrote:
 On windows XP with DMD 2.057, I get
 Queue1: deadlock
 Queue2: works
 Queue3: works

 Yes, I posted another (much shorter) post describing the issue with
 Queue1. In short, since Queue1 is a synchronized class, the
 constructor is synchronized (which is mostly worthless). As a
 consequence, when I replace the lock in the middle of the function,
 bad things happen when the runtime tries to unlock the lock at the end
 and sees the new lock.

 This version of Queue1 shows a very hacky way to get around this:
 ---
 synchronized class Queue1 {private: =A0 bool _work; =A0 Condition
 _cond;public: =A0 this() { =A0 =A0 =A0 auto lock =3D new Mutex(this); /=



/
 initialize the monitor for this object so we can use the same lock in
 the Condition =A0 =A0 =A0 lock.lock(); // HACK: acquire the lock so we =



can
 unlock it at the end of the function =A0 =A0 =A0 _cond =3D new
 Condition(lock); =A0 =A0 =A0 _work =3D false; =A0 } =A0 void doWork() {
 while(!_work) (cast()_cond).wait(); =A0 =A0 =A0 _work =3D false;
 writeln("did work"); =A0 =A0 =A0 return; =A0 } =A0 void addWork() { =A0=



 =A0 =A0 _work
 =3D true; =A0 =A0 =A0 (cast()_cond).notify(); =A0 =A0 =A0 writeln("adde=



d work");
 }}---
 Queue2 looks like a bug where GDC is acquiring all locks twice in
 synchronized functions, and since the condition variable only unlocks
 the lock once, a deadlock results. I'll get a bug report up about it
 shortly.

 Gah, my hacked/fixed Queue1 got garbled:
 ---
 synchronized class Queue1 {
 private:
 =A0 =A0bool _work;
 =A0 =A0Condition _cond;
 public:
 =A0 =A0this() {
 =A0 =A0 =A0 =A0 auto lock =3D new Mutex(this); // initialize the monitor=


 for this object
 This assumes that there is no existing monitor for the object. =A0At best=

 you'll get a memory leak here.

Then is there any way to safely use this sort of idiom? Putting it on
the first line of the constructor was the earliest way I could see to
try to swap the lock.

Sean Kelly <sean invisibleduck.org> writes:

On Jan 4, 2012, at 2:55 PM, Andrew Wiley wrote:

 On Wed, Jan 4, 2012 at 2:12 PM, Sean Kelly <sean invisibleduck.org> =

wrote:
=20
=20
 This assumes that there is no existing monitor for the object.  At =


best you'll get a memory leak here.
=20
 Then is there any way to safely use this sort of idiom? Putting it on
 the first line of the constructor was the earliest way I could see to
 try to swap the lock.

Not currently.  The relevant ctor for Mutex actually has an =
"assert(o.__monitor is null)" clause in it that simply never triggers =
because we ship a "release" build (I hate that label).  The problem is =
that replacing an existing monitor means a race condition, since other =
threads might be trying to lock it or are waiting on it at the time.  =
The real issue is that the ctor for a synchronized module shouldn't be =
synchronized, but I'll grant that it's easier to fix this in =
user/library code than sort out a compiler fix.  What you can do is:
---
extern (C) void _d_monitordelete(Object h, bool det);
synchronized class Queue1 {
private:
   bool _work;
   Condition _cond;
public:
   this() {
        _d_monitordelete(this, true);
        auto lock =3D new Mutex(this); // initialize the monitor for =
this object
       // so we can use the same lock in the Condition
       lock.lock(); // HACK: acquire the lock so we can unlock it
       // at the end of the function
       _cond =3D new Condition(lock);
       _work =3D false;
   }
   ...
}
---
Obviously not ideal since it requires a lot of hackery, but it should =
work as desired.=

"Steven Schveighoffer" <schveiguy yahoo.com> writes:

On Wed, 04 Jan 2012 19:12:04 -0500, Sean Kelly <sean invisibleduck.org>  
wrote:

 On Jan 4, 2012, at 2:55 PM, Andrew Wiley wrote:

 On Wed, Jan 4, 2012 at 2:12 PM, Sean Kelly <sean invisibleduck.org>  
 wrote:
 This assumes that there is no existing monitor for the object.  At  
 best you'll get a memory leak here.

 Then is there any way to safely use this sort of idiom? Putting it on
 the first line of the constructor was the earliest way I could see to
 try to swap the lock.

 Not currently.  The relevant ctor for Mutex actually has an  
 "assert(o.__monitor is null)" clause in it that simply never triggers  
 because we ship a "release" build (I hate that label).  The problem is  
 that replacing an existing monitor means a race condition, since other  
 threads might be trying to lock it or are waiting on it at the time.   
 The real issue is that the ctor for a synchronized module shouldn't be  
 synchronized, but I'll grant that it's easier to fix this in  
 user/library code than sort out a compiler fix.  What you can do is:
 ---
 extern (C) void _d_monitordelete(Object h, bool det);
 synchronized class Queue1 {
 private:
    bool _work;
    Condition _cond;
 public:
    this() {
         _d_monitordelete(this, true);
         auto lock = new Mutex(this); // initialize the monitor for this  
 object
        // so we can use the same lock in the Condition
        lock.lock(); // HACK: acquire the lock so we can unlock it
        // at the end of the function
        _cond = new Condition(lock);
        _work = false;
    }
    ...
 }
 ---
 Obviously not ideal since it requires a lot of hackery, but it should  
 work as desired.

Can Mutex do this?

I'm not sure a synchronized class shouldn't be locked in the ctor, it's  
entirely feasible for a synchronized ctor to place its this reference in  
some global location for other threads to try and lock.

But if Mutex can check if there's an existing lock, make sure it's  
unlocked, then replace it with a new one, I think it should be sound, as  
long as you do it in the ctor before exposing the 'this' reference  
somewhere.

-Steve

Sean Kelly <sean invisibleduck.org> writes:

There's still a race condition between unlocking the monitor and freeing the=
 memory.=20

Sent from my iPhone

On Jan 7, 2012, at 7:25 AM, "Steven Schveighoffer" <schveiguy yahoo.com> wro=
te:

 On Wed, 04 Jan 2012 19:12:04 -0500, Sean Kelly <sean invisibleduck.org> wr=

ote:
=20
 On Jan 4, 2012, at 2:55 PM, Andrew Wiley wrote:
=20
 On Wed, Jan 4, 2012 at 2:12 PM, Sean Kelly <sean invisibleduck.org> wrot=



e:
=20
=20
 This assumes that there is no existing monitor for the object.  At best=




 you'll get a memory leak here.
=20
 Then is there any way to safely use this sort of idiom? Putting it on
 the first line of the constructor was the earliest way I could see to
 try to swap the lock.

=20
 Not currently.  The relevant ctor for Mutex actually has an "assert(o.__m=


onitor is null)" clause in it that simply never triggers because we ship a "=
release" build (I hate that label).  The problem is that replacing an existi=
ng monitor means a race condition, since other threads might be trying to lo=
ck it or are waiting on it at the time.  The real issue is that the ctor for=
 a synchronized module shouldn't be synchronized, but I'll grant that it's e=
asier to fix this in user/library code than sort out a compiler fix.  What y=
ou can do is:
 ---
 extern (C) void _d_monitordelete(Object h, bool det);
 synchronized class Queue1 {
 private:
   bool _work;
   Condition _cond;
 public:
   this() {
        _d_monitordelete(this, true);
        auto lock =3D new Mutex(this); // initialize the monitor for this o=


bject
       // so we can use the same lock in the Condition
       lock.lock(); // HACK: acquire the lock so we can unlock it
       // at the end of the function
       _cond =3D new Condition(lock);
       _work =3D false;
   }
   ...
 }
 ---
 Obviously not ideal since it requires a lot of hackery, but it should wor=


k as desired.
=20
 Can Mutex do this?
=20
 I'm not sure a synchronized class shouldn't be locked in the ctor, it's en=

tirely feasible for a synchronized ctor to place its this reference in some g=
lobal location for other threads to try and lock.
=20
 But if Mutex can check if there's an existing lock, make sure it's unlocke=

d, then replace it with a new one, I think it should be sound, as long as yo=
u do it in the ctor before exposing the 'this' reference somewhere.
=20
 -Steve

"Steven Schveighoffer" <schveiguy yahoo.com> writes:

On Sat, 07 Jan 2012 11:08:51 -0500, Sean Kelly <sean invisibleduck.org>  
wrote:

 There's still a race condition between unlocking the monitor and freeing  
 the memory.

Wouldn't that result in a segfault or other error?  How is that worse than  
the current state of affairs?

-Steve

Sean Kelly <sean invisibleduck.org> writes:

Right now, Mutex will assert if you assign it to an object with a monitor. O=
r it would with a non-release build. That's the only entirely safe thing to d=
o.=20

Sent from my iPhone

On Jan 7, 2012, at 9:27 AM, "Steven Schveighoffer" <schveiguy yahoo.com> wro=
te:

 On Sat, 07 Jan 2012 11:08:51 -0500, Sean Kelly <sean invisibleduck.org> wr=

ote:
=20
 There's still a race condition between unlocking the monitor and freeing t=


he memory.
=20
 Wouldn't that result in a segfault or other error?  How is that worse than=

 the current state of affairs?
=20
 -Steve

"Steven Schveighoffer" <schveiguy yahoo.com> writes:

On Sat, 07 Jan 2012 18:07:33 -0500, Sean Kelly <sean invisibleduck.org>  
wrote:

 Right now, Mutex will assert if you assign it to an object with a  
 monitor. Or it would with a non-release build.

In other words, it doesn't :)

-Steve

Sean Kelly <sean invisibleduck.org> writes:

Which goes back to my argument that we should ship both checked and unchecke=
d (release and not) builds and the compiler should link the proper one.=20

Sent from my iPhone

On Jan 9, 2012, at 6:01 AM, "Steven Schveighoffer" <schveiguy yahoo.com> wro=
te:

 On Sat, 07 Jan 2012 18:07:33 -0500, Sean Kelly <sean invisibleduck.org> wr=

ote:
=20
 Right now, Mutex will assert if you assign it to an object with a monitor=


. Or it would with a non-release build.
=20
 In other words, it doesn't :)
=20
 -Steve

"Steven Schveighoffer" <schveiguy yahoo.com> writes:

On Mon, 09 Jan 2012 09:41:40 -0500, Sean Kelly <sean invisibleduck.org>  
wrote:

 Which goes back to my argument that we should ship both checked and  
 unchecked (release and not) builds and the compiler should link the  
 proper one.

Or you make it a release-mode check.  I can't imagine this being an  
inner-loop operation.

But my point was, it's already unsafe.  We've not had horrible issues with  
it in its current state.  Is it worse to allow replacing the monitor?  If  
you take the right steps (i.e. do it first line of the ctor), it's not  
going to result in a race condition.

-Steve

Sean Kelly <sean invisibleduck.org> writes:

I know. But once I add the ability to replace a mutex there's no way to dete=
ct how it's being used. Though as its in core I guess the user should simply=
 be trusted to do the right thing.=20

Sent from my iPhone

On Jan 9, 2012, at 6:57 AM, "Steven Schveighoffer" <schveiguy yahoo.com> wro=
te:

 On Mon, 09 Jan 2012 09:41:40 -0500, Sean Kelly <sean invisibleduck.org> wr=

ote:
=20
 Which goes back to my argument that we should ship both checked and unche=


cked (release and not) builds and the compiler should link the proper one.
=20
 Or you make it a release-mode check.  I can't imagine this being an inner-=

loop operation.
=20
 But my point was, it's already unsafe.  We've not had horrible issues with=

 it in its current state.  Is it worse to allow replacing the monitor?  If y=
ou take the right steps (i.e. do it first line of the ctor), it's not going t=
o result in a race condition.
=20
 -Steve

Andrew Wiley <wiley.andrew.j gmail.com> writes:

On Mon, Jan 9, 2012 at 8:41 AM, Sean Kelly <sean invisibleduck.org> wrote:
 Which goes back to my argument that we should ship both checked and unchecked
(release and not) builds and the compiler should link the proper one.

 Sent from my iPhone

I agree. This is common in the C/C++ world, and it seems even more
important in D since we have so many DbC features that disappear
completely in release-mode binaries.
If this isn't supported, DbC is worthless in library code.

Andrew Wiley <wiley.andrew.j gmail.com> writes:

On Wed, Jan 4, 2012 at 6:12 PM, Sean Kelly <sean invisibleduck.org> wrote:
 On Jan 4, 2012, at 2:55 PM, Andrew Wiley wrote:

 On Wed, Jan 4, 2012 at 2:12 PM, Sean Kelly <sean invisibleduck.org> wrot=


e:
 This assumes that there is no existing monitor for the object. =A0At be=



st you'll get a memory leak here.
 Then is there any way to safely use this sort of idiom? Putting it on
 the first line of the constructor was the earliest way I could see to
 try to swap the lock.

 Not currently. =A0The relevant ctor for Mutex actually has an "assert(o._=

_monitor is null)" clause in it that simply never triggers because we ship =
a "release" build (I hate that label). =A0The problem is that replacing an =
existing monitor means a race condition, since other threads might be tryin=
g to lock it or are waiting on it at the time. =A0The real issue is that th=
e ctor for a synchronized module shouldn't be synchronized, but I'll grant =
that it's easier to fix this in user/library code than sort out a compiler =
fix. =A0What you can do is:
 ---
 extern (C) void _d_monitordelete(Object h, bool det);
 synchronized class Queue1 {
 private:
 =A0 bool _work;
 =A0 Condition _cond;
 public:
 =A0 this() {
 =A0 =A0 =A0 =A0_d_monitordelete(this, true);
 =A0 =A0 =A0 =A0auto lock =3D new Mutex(this); // initialize the monitor f=

or this object
 =A0 =A0 =A0 // so we can use the same lock in the Condition
 =A0 =A0 =A0 lock.lock(); // HACK: acquire the lock so we can unlock it
 =A0 =A0 =A0 // at the end of the function
 =A0 =A0 =A0 _cond =3D new Condition(lock);
 =A0 =A0 =A0 _work =3D false;
 =A0 }
 =A0 ...
 }
 ---
 Obviously not ideal since it requires a lot of hackery, but it should wor=

k as desired.

I haven't filed a bug yet about constructors for synchronized classes
being synchronized. Should I?
I believe it's wrong and unhelpful, but I haven't really gotten any
feedback and I'm afraid I may have missed something.

Sean Kelly <sean invisibleduck.org> writes:

It's just like ctors of invariant classes being invariant. Initialization sh=
ould be exempt from the rules surrounding other accesses.=20

Sent from my iPhone

On Jan 4, 2012, at 5:06 PM, Andrew Wiley <wiley.andrew.j gmail.com> wrote:

 On Wed, Jan 4, 2012 at 6:12 PM, Sean Kelly <sean invisibleduck.org> wrote:=

 On Jan 4, 2012, at 2:55 PM, Andrew Wiley wrote:
=20
 On Wed, Jan 4, 2012 at 2:12 PM, Sean Kelly <sean invisibleduck.org> wrot=



e:
=20
=20
 This assumes that there is no existing monitor for the object.  At best=




 you'll get a memory leak here.
=20
 Then is there any way to safely use this sort of idiom? Putting it on
 the first line of the constructor was the earliest way I could see to
 try to swap the lock.

=20
 Not currently.  The relevant ctor for Mutex actually has an "assert(o.__m=


onitor is null)" clause in it that simply never triggers because we ship a "=
release" build (I hate that label).  The problem is that replacing an existi=
ng monitor means a race condition, since other threads might be trying to lo=
ck it or are waiting on it at the time.  The real issue is that the ctor for=
 a synchronized module shouldn't be synchronized, but I'll grant that it's e=
asier to fix this in user/library code than sort out a compiler fix.  What y=
ou can do is:
 ---
 extern (C) void _d_monitordelete(Object h, bool det);
 synchronized class Queue1 {
 private:
   bool _work;
   Condition _cond;
 public:
   this() {
        _d_monitordelete(this, true);
        auto lock =3D new Mutex(this); // initialize the monitor for this o=


bject
       // so we can use the same lock in the Condition
       lock.lock(); // HACK: acquire the lock so we can unlock it
       // at the end of the function
       _cond =3D new Condition(lock);
       _work =3D false;
   }
   ...
 }
 ---
 Obviously not ideal since it requires a lot of hackery, but it should wor=


k as desired.
=20
 I haven't filed a bug yet about constructors for synchronized classes
 being synchronized. Should I?
 I believe it's wrong and unhelpful, but I haven't really gotten any
 feedback and I'm afraid I may have missed something.