Download Reference Manual
The Developer's Library for D
About Wiki Forums Source Search Contact

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