[RFC][plugin]: fix concurrency problems in PAV.java
Denis Lila
dlila at redhat.com
Thu Apr 14 08:43:52 PDT 2011
> It is not irrelevant to strongly type a container.
What I meant by irrelevant is that NetxPanel is a subclass, so it
adds to the implementation of AppletPanel. We don't need any
of what it adds, because the only reason we have appletPanels
is so we can implement getApplet(String) and getApplets().
These methods only need a container whose elements have an
applet field, and Vector<AppletPanel> is enough for that.
> Strong typing will catch errors at compile time whereas weak
> typing will show them at runtime.
The reason for that is because when a container is weakly typed
we may put bad data in it and get ClassCastExceptions when we
manually cast. In this case, however, we never do any casts
because the type is exactly as strong as it needs to be,
because all we need is the ".applet" member, and AppletPanel
has it.
Do you still think I should change it to NetxPanel?
Regards,
Denis.
----- Original Message -----
> * Denis Lila <dlila at redhat.com> [2011-04-14 10:07]:
> > > NetxPanel inherits from AVP, so why is the cast needed? Does
> > > NetxPanel
> > > override the method?
> >
> > Yes, NetxPanel overrides the 3 methods that we need the casts for.
> > Since they're protected, but NetxPanel is in a different package,
> > the compiler gives an error.
> >
>
> Ah okay then, thats what I suspected too.
>
> > > > That said, I wouldn't mind changing the type of appletPanels
> > > > to Vector<AVP>. Do you still think I should?
> > > >
> > >
> > > Actually, perhaps we should go one step up and just change it to
> > > NetxPanel? We only store those in the vector anyway.
> >
> > We could, but I would rather not because we only use that vector
> > for the "applet" field of the AppletPanel class. We don't need
> > a NetxPanel for that. Using a NetxPanel would add irrelevant
> > details.
> >
>
>
> Cheers,
> Deepak
>
> > Regards,
> > Denis.
> >
> > ----- Original Message -----
> > > * Denis Lila <dlila at redhat.com> [2011-04-14 09:45]:
> > > > > Instead of doing this, it would be easier to change the type
> > > > > of
> > > > > appletPanels to Vector<AppletViewerPanel> , then the casting
> > > > > is
> > > > > not
> > > > > needed as AVP inherits from AP.
> > > >
> > > > The casting to AppletViewerPanel is not caused by appletPanels.
> > > > It
> > > > is caused by the "panel" member. It used to be declared as an
> > > > AVP,
> > > > then I changed it to a NetxPanel, which made the 3 new casts
> > > > necessary (but it eliminated many more casts than it added).
> > > >
> > >
> >
> > >
> >
> > >
> > > Cheers,
> > > Deepak
> > >
> > > > Thank you,
> > > > Denis.
> > > >
> > > > ----- Original Message -----
> > > > > * Denis Lila <dlila at redhat.com> [2011-04-13 15:10]:
> > > > > > > If a patch akin to
> > > > > > > http://hg.openjdk.java.net/jdk6/jdk6/langtools/rev/5c2858bccb3f
> > > > > > > goes in, I will not be responsible for my actions.
> > > > > >
> > > > > > I definitely don't want that ;-).
> > > > > >
> > > > > > Attached is the second patch. It's mostly cleanup, and it
> > > > > > also uses "pav" in destroyApplet, instead of calling
> > > > > > get(id) again and again.
> > > > > >
> > > > > > Ok to push?
> > > > > >
> > > > >
> > > > > ...
> > > > > ...
> > > > >
> > > > > <snip>
> > > > > >
> > > > > > PluginDebug.debug("getCachedImageRef() getting
> > > > > > img
> > > > > > from
> > > > > > URL = ", url);
> > > > > > @@ -910,8 +815,8 @@
> > > > > > SocketPermission panelSp =
> > > > > > new
> > > > > > SocketPermission(panel.getCodeBase().getHost(),
> > > > > > "connect");
> > > > > > synchronized(appletPanels) {
> > > > > > - for (Enumeration e = appletPanels.elements();
> > > > > > e.hasMoreElements();) {
> > > > > > - AppletPanel p = (AppletPanel) e.nextElement();
> > > > > > + for (Enumeration<AppletPanel> e = appletPanels.elements();
> > > > > > e.hasMoreElements();) {
> > > > > > + AppletPanel p = e.nextElement();
> > > > > > String param = p.getParameter("name");
> > > > > > if (param != null) {
> > > > > > param = param.toLowerCase();
> > > > > > @@ -1591,15 +1496,15 @@
> > > > > > * at the same time.
> > > > > > */
> > > > > > try {
> > > > > > - panel.joinAppletThread();
> > > > > > - panel.release();
> > > > > > + ((AppletViewerPanel)panel).joinAppletThread();
> > > > > > + ((AppletViewerPanel)panel).release();
> > > > > > } catch (InterruptedException e) {
> > > > > > return; // abort the reload
> > > > > > }
> > > > > >
> > > > > > AccessController.doPrivileged(new
> > > > > > PrivilegedAction<Void>()
> > > > > > {
> > > > > > public Void run() {
> > > > > > - panel.createAppletThread();
> > > > > > + ((AppletViewerPanel)panel).createAppletThread();
> > > > > > return null;
> > > > > > }
> > > > >
> > > > >
> > > >
More information about the distro-pkg-dev
mailing list