[RFC][Icedtea-Web]: Fix bug where you can clear cache while plugin is in use.

Andrew Su asu at redhat.com
Wed Mar 16 11:56:08 PDT 2011



----- Original Message -----
> On 10:23 Wed 16 Mar , Andrew Su wrote:
> >
> >
> > ----- Original Message -----
> > > On 03/15/2011 04:29 PM, Andrew Su wrote:
> > > > Hello Denis,
> > > >
> > > > ----- Original Message -----
> > > >> Hi.
> > > >>
> > > >>>>> - Is it possible to get a race condition when calling
> > > >>>>> markNetxRunning in JNLPRuntime. Since this now affects the
> > > >>>>> plugin and not just javaws, this might be called multiple
> > > >>>>> times by different threads? if so, I would synchronize it
> > > >>>>> and
> > > >>>>> since subsequent calls will instantly return if the first
> > > >>>>> call succeeded in getting the lock, so there won't be much
> > > >>>>> performance hit.
> > > >>
> > > >> I think that's a very good idea (making markNetxRunning
> > > >> synchronized). I don't think we need to worry about performance
> > > >> at
> > > >> all, seeing how rarely this function is called.
> > > >
> > > > It does get called every time it is about to launch an
> > > > applet/application. but as a counter measure it will return
> > > > right
> > > > away if we obtained the lock already.
> > > >
> > >
> > > Without appropriate synchronization, another thread may not see
> > > that
> > > it's null already. Being a shared variable that's being read and
> > > set
> > > by
> > > multiple threads, it needs appropriate synchronization.
> > >
> > > Take a look at
> > > http://download.oracle.com/javase/tutorial/essential/concurrency/memconsist.html
> > >
> >
> > Yes, that is what I meant when I said that I will synchronize it.
> > The check for null is so we don't execute the rest of the body not
> > for synchronization purposes.
> >
> > Well, here's the updated patch.
> >
> > Cheers,
> >   Andrew
> >
> > > Cheers,
> > > Omair
> 
> Should markNetxRunning be able to run at the same time as
> markNetxStopped?
> You only have the former synchronized, and not the latter. I think
> they
> both should be.

Hi Andrew,
They very well shouldn't, since markNetxStopped is only executed once at shutdown (and it is added after everything in markNetxRunning is done), so the way this is being done is a non-issue imo. There should be no other places where markNetxStopped should be called.

Regards,
  Andrew

> 
> > diff -r 2d359e723fef netx/net/sourceforge/jnlp/Launcher.java
> > --- a/netx/net/sourceforge/jnlp/Launcher.java Wed Mar 16 12:01:58
> > 2011 +0100
> > +++ b/netx/net/sourceforge/jnlp/Launcher.java Wed Mar 16 10:21:21
> > 2011 -0400
> > @@ -28,7 +28,6 @@
> >  import java.net.InetAddress;
> >  import java.net.URL;
> >  import java.net.UnknownHostException;
> > -import java.nio.channels.FileChannel;
> >  import java.nio.channels.FileLock;
> >  import java.util.LinkedList;
> >  import java.util.List;
> > @@ -37,7 +36,6 @@
> >  import net.sourceforge.jnlp.cache.CacheUtil;
> >  import net.sourceforge.jnlp.cache.ResourceTracker;
> >  import net.sourceforge.jnlp.cache.UpdatePolicy;
> > -import net.sourceforge.jnlp.config.DeploymentConfiguration;
> >  import net.sourceforge.jnlp.runtime.AppThreadGroup;
> >  import net.sourceforge.jnlp.runtime.AppletInstance;
> >  import net.sourceforge.jnlp.runtime.ApplicationInstance;
> > @@ -45,7 +43,6 @@
> >  import net.sourceforge.jnlp.runtime.JNLPRuntime;
> >  import net.sourceforge.jnlp.services.InstanceExistsException;
> >  import net.sourceforge.jnlp.services.ServiceUtil;
> > -import net.sourceforge.jnlp.util.FileUtils;
> >
> >  import javax.swing.SwingUtilities;
> >  import javax.swing.text.html.parser.ParserDelegator;
> > @@ -214,6 +211,8 @@
> >              }
> >          }
> >
> > + JNLPRuntime.markNetxRunning();
> > +
> >          if (file instanceof PluginBridge && cont != null)
> >              tg = new TgThread(file, cont, true);
> >          else if (cont == null)
> > @@ -387,13 +386,6 @@
> >          if (!file.isApplication())
> >              throw launchError(new LaunchException(file, null,
> >              R("LSFatal"), R("LCClient"), R("LNotApplication"),
> >              R("LNotApplicationInfo")));
> >
> > - markNetxRunning();
> > - Runtime.getRuntime().addShutdownHook(new Thread() {
> > - public void run() {
> > - markNetxStopped();
> > - }
> > - });
> > -
> >          try {
> >
> >              try {
> > @@ -699,66 +691,6 @@
> >      }
> >
> >      /**
> > - * Indicate that netx is running by creating the {@link
> > JNLPRuntime#INSTANCE_FILE} and
> > - * acquiring a shared lock on it
> > - */
> > - private void markNetxRunning() {
> > - try {
> > - String message = "This file is used to check if netx is running";
> > -
> > - File netxRunningFile = new File(JNLPRuntime.getConfiguration()
> > - .getProperty(DeploymentConfiguration.KEY_USER_NETX_RUNNING_FILE));
> > - if (!netxRunningFile.exists()) {
> > - FileUtils.createParentDir(netxRunningFile);
> > - FileUtils.createRestrictedFile(netxRunningFile, true);
> > - FileOutputStream fos = new FileOutputStream(netxRunningFile);
> > - try {
> > - fos.write(message.getBytes());
> > - } finally {
> > - fos.close();
> > - }
> > - }
> > -
> > - FileInputStream is = new FileInputStream(netxRunningFile);
> > - FileChannel channel = is.getChannel();
> > - fileLock = channel.tryLock(0, Long.MAX_VALUE, true);
> > - if (fileLock != null && fileLock.isShared()) {
> > - if (JNLPRuntime.isDebug()) {
> > - System.out.println("Acquired shared lock on " +
> > - netxRunningFile.toString() + " to indicate javaws is running");
> > - }
> > - } else {
> > - fileLock = null;
> > - }
> > - } catch (IOException e) {
> > - e.printStackTrace();
> > - }
> > -
> > - }
> > -
> > - /**
> > - * Indicate that netx is stopped by releasing the shared lock on
> > - * {@link JNLPRuntime#INSTANCE_FILE}.
> > - */
> > - private void markNetxStopped() {
> > - if (fileLock == null) {
> > - return;
> > - }
> > - try {
> > - fileLock.release();
> > - fileLock.channel().close();
> > - fileLock = null;
> > - if (JNLPRuntime.isDebug()) {
> > - String file = JNLPRuntime.getConfiguration()
> > - .getProperty(DeploymentConfiguration.KEY_USER_NETX_RUNNING_FILE);
> > - System.out.println("Release shared lock on " + file);
> > - }
> > - } catch (IOException e) {
> > - e.printStackTrace();
> > - }
> > - }
> > -
> > - /**
> >       * Do hacks on per-application level to allow different
> >       AppContexts to work
> >       *
> >       * @see JNLPRuntime#doMainAppContextHacks
> > diff -r 2d359e723fef
> > netx/net/sourceforge/jnlp/runtime/JNLPRuntime.java
> > --- a/netx/net/sourceforge/jnlp/runtime/JNLPRuntime.java Wed Mar 16
> > 12:01:58 2011 +0100
> > +++ b/netx/net/sourceforge/jnlp/runtime/JNLPRuntime.java Wed Mar 16
> > 10:21:21 2011 -0400
> > @@ -19,6 +19,8 @@
> >  import java.io.*;
> >  import java.net.Authenticator;
> >  import java.net.ProxySelector;
> > +import java.nio.channels.FileChannel;
> > +import java.nio.channels.FileLock;
> >  import java.awt.*;
> >  import java.text.*;
> >  import java.util.*;
> > @@ -122,6 +124,9 @@
> >      /** contains the arguments passed to the jnlp runtime */
> >      private static List<String> initialArguments;
> >
> > + /** lock to file that indicates NetX is running */
> > + private static FileLock fileLock = null;
> > +
> >      public static final String STDERR_FILE = "java.stderr";
> >      public static final String STDOUT_FILE = "java.stdout";
> >
> > @@ -627,4 +632,69 @@
> >          return initialArguments;
> >      }
> >
> > + /**
> > + * Indicate that netx is running by creating the {@link
> > JNLPRuntime#INSTANCE_FILE} and
> > + * acquiring a shared lock on it
> > + */
> > + public synchronized static void markNetxRunning() {
> > + if (fileLock != null) return;
> > + try {
> > + String message = "This file is used to check if netx is running";
> > +
> > + File netxRunningFile = new File(JNLPRuntime.getConfiguration()
> > + .getProperty(DeploymentConfiguration.KEY_USER_NETX_RUNNING_FILE));
> > + if (!netxRunningFile.exists()) {
> > + FileUtils.createParentDir(netxRunningFile);
> > + FileUtils.createRestrictedFile(netxRunningFile, true);
> > + FileOutputStream fos = new FileOutputStream(netxRunningFile);
> > + try {
> > + fos.write(message.getBytes());
> > + } finally {
> > + fos.close();
> > + }
> > + }
> > +
> > + FileInputStream is = new FileInputStream(netxRunningFile);
> > + FileChannel channel = is.getChannel();
> > + fileLock = channel.tryLock(0, Long.MAX_VALUE, true);
> > + if (fileLock != null && fileLock.isShared()) {
> > + if (JNLPRuntime.isDebug()) {
> > + System.out.println("Acquired shared lock on " +
> > + netxRunningFile.toString() + " to indicate javaws is running");
> > + }
> > + } else {
> > + fileLock = null;
> > + }
> > + } catch (IOException e) {
> > + e.printStackTrace();
> > + }
> > +
> > + Runtime.getRuntime().addShutdownHook(new Thread() {
> > + public void run() {
> > + markNetxStopped();
> > + }
> > + });
> > + }
> > +
> > + /**
> > + * Indicate that netx is stopped by releasing the shared lock on
> > + * {@link JNLPRuntime#INSTANCE_FILE}.
> > + */
> > + private static void markNetxStopped() {
> > + if (fileLock == null) {
> > + return;
> > + }
> > + try {
> > + fileLock.release();
> > + fileLock.channel().close();
> > + fileLock = null;
> > + if (JNLPRuntime.isDebug()) {
> > + String file = JNLPRuntime.getConfiguration()
> > + .getProperty(DeploymentConfiguration.KEY_USER_NETX_RUNNING_FILE);
> > + System.out.println("Release shared lock on " + file);
> > + }
> > + } catch (IOException e) {
> > + e.printStackTrace();
> > + }
> > + }
> >  }
> 
> 
> --
> 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