[RFC][plugin]: fix fixme

[Deepak Bhole]

* Dr Andrew John Hughes <ahughes at redhat.com> [2011-03-30 20:22]:
> On 19:27 Wed 30 Mar     , Deepak Bhole wrote:
> > * Denis Lila <dlila at redhat.com> [2011-03-30 18:51]:
> > > > 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 think Tom originally intended for it to be singleton. However
> > the surrounding code written later made it such. Frankly, I haven't
> > looked into it much (outside of from a security perspective) because
> > there have been almost no bugs (if any) in the liveconnect part of the
> > code (to which PluginObjectStore belongs). Since it ain't broke, I don't
> > want to try to fix it and introduce potential issues :)
> > 
> 
> Then, given it's no guaranteed to be a singleton, I prefer the version originally
> given for safety.
>

Fair enough. I am okay with that as well!

Cheers,
Deepak
 
> > Cheers,
> > Deepak
> > 
> > > Just to merge the threads: (we're pretty much saying the same thing :-)
> > > 
> > > http://mail.openjdk.java.net/pipermail/distro-pkg-dev/2011-March/013257.html
> > > 
> > > ----- Original Message -----
> > > > 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.
> > > > >
> > > > 
> > > 
> > > 
> > > > 
> > > > > > 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
> 
> -- 
> 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