Ticket #1171 (closed defect: fixed)

Opened 2 months ago

Last modified 1 month ago

Usage of 'synchronized' in tango.core.Thread.ThreadGroup

Reported by:	LeoD	Assigned to:	sean
Priority:	normal	Milestone:	0.99.8
Component:	Core Functionality	Version:	0.99.6 Jeff
Keywords:		Cc:

Description

In the ThreadGroup? class, synchronized is being used without any parameters, for example:

    final void add( Thread t )
    in
    {
        assert( t );
    }
    body
    {
        synchronized
        {
            m_all[t] = t;
        }
    }

    // ...

    final void remove( Thread t )
    in
    {
        assert( t );
    }
    body
    {
        synchronized
        {
            m_all.remove( t );
        }
    }

But, according to the current spec, each 'synchronized' block that doesn't have a parameter gets its own lock - which doesn't seem to be what is needed here.

I think all 'synchronized' blocks should be changed to 'synchronized(this)'.

Change History

07/10/08 06:47:41 changed by larsivi

milestone changed from 0.99.7 to 0.99.8.

07/25/08 14:41:22 changed by sean

status changed from new to assigned.

The spec is wrong, but I'll verify with a debugger to be sure.

07/25/08 15:27:10 changed by sean

Holy crap you're right. This is the most utterly broken part of D that I've ever encountered. I'll fix it immediately.

07/25/08 15:30:47 changed by sean

status changed from assigned to closed.
resolution set to fixed.

(In [3800]) Whoever came up with the rule that "synchronized" without a "(this)" inside a class member function would synchronize on the global monitor instead of the instance monitor was smoking crack. This is completely ridiculous. I've fixed the instances where we were using unlabeled "synchronized" inside of the ThreadGroup? object. This closes #1171

Ticket Navigation