Some Issues With Synchronized

[Sean Kelly]

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 at gmail.com> wrote:
>> On Fri, Dec 23, 2011 at 1:25 AM, Somedude <lovelydear at 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:   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       lock.lock(); // HACK: acquire the lock so we can
>> unlock it at the end of the function       _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");
>> }}---
>> 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:
>    bool _work;
>    Condition _cond;
> public:
>    this() {
>         auto lock = 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.