Some Issues With Synchronized

[Andrew Wiley]

On Wed, Jan 4, 2012 at 6:12 PM, Sean Kelly <sean at 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 at 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.

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.