[RFC][plugin]: fix fixme

Dr Andrew John Hughes ahughes at redhat.com
Wed Mar 30 15:35:04 PDT 2011


On 16:34 Wed 30 Mar     , Deepak Bhole wrote:
> * Dr Andrew John Hughes <ahughes at redhat.com> [2011-03-30 16:22]:
> > On 10:46 Wed 30 Mar     , Deepak Bhole wrote:
> > 
> > snip...
> > 
> > > Why not synchronize the whole function? The whole body is synchronized
> > > on a static variable anyway. I think the multi-threaded map access is a
> > > genuine potential problem. Making the reference and unreference
> > > functions synchronized and keeping everything else as is should be
> > > enough to address that.
> > > 
> > 
> > They aren't the same thing.  Making the methods synchronised is equivalent
> > to wrapping the body in synchronized (this) and wouldn't protect against
> > access to the maps from another instance.
> >
> 
> There is only 1 object store instance at any given time.
> 

That's not obvious in the patch.

So the class is a singleton?  Is this enforced by the constructor?
Is there a reason for having instance methods at all and not just having everything
static?

> > I don't see the immediate reason for three maps, but I don't know the rest
> > of this code that well.  Can you explain how 'multi-threaded map access is
> > a genuine potential problem'?  nextID is not going to take a long amount of
> > time so keeping objects locked over that period doesn't seem an issue.
> >
> 
> I meant it is a problem in the current code which does no
> synchronization.
> 

Well yeah, that's the original point of the patch surely...

> Deepak
> 

-- 
Andrew :)

Free Java Software Engineer
Red Hat, Inc. (http://www.redhat.com)

Support Free Java!
Contribute to GNU Classpath and IcedTea
http://www.gnu.org/software/classpath
http://icedtea.classpath.org
PGP Key: F5862A37 (https://keys.indymedia.org/)
Fingerprint = EA30 D855 D50F 90CD F54D  0698 0713 C3ED F586 2A37



More information about the distro-pkg-dev mailing list