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

[Andrew Su]

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.


> 
> > > > - We try to attain the lock but fails if javaws -Xclearcache is
> > > > in
> > > > progress, as calling clear cache it holds an exclusive lock
> > > > (which
> > > > is
> > > > right). However we would proceed as normal (by normal I mean it
> > > > can
> > > > be
> > > > downloading new jars for applets and such) without having a lock
> > > > to
> > > > indicate that we are running. Which can still lead to the
> > > > problem
> > > > we
> > > > are
> > > > trying to fix. To remedy this we should either make it wait
> > > > until
> > > > the
> > > > lock becomes free. (and thus we need to synchronize it if this
> > > > can
> > > > be
> > > > called by multiple threads.)
> 
> At first I was afraid that blocking would limit the number of jnlp
> applications that can be running at a time to one, but in mark running
> we get a shared lock, while in clear cache we get an exclusive lock,
> so
> as long as we have shared locks, blocking should be fine (but I think
> mark running should be the only thing that blocks - not clear cache.
> clear
> cache should just fail if it can't get the lock). Also, if we're going
> to
> block, we need to release the lock after the cache is deleted. Right
> now
> we release it right after okToClearCache returns.

Ah yes, I missed that. It is ok if we don't release the lock from okToClearCache since these calls should only happen when either the jvm is shuttingdown or when we call to clear cache which will return after it does its check + clear if allowed.

> 
> There is one more thing. The FileLock spec says
> "Some platforms do not support shared locks, in which case a request
> for
> a shared lock is automatically converted into a request for an
> exclusive
> lock". Is there any risk of this happening? If it does happen, then
> to get the behaviour we want we'd need something like this:
> Introduce a new file lock CL (stands for CacheLock)
> and then when clearing the cache do:

In the event that an exclusive lock is being held, other instances won't be able to obtain a shared lock.
Similarly, if a shared lock is being held, an exclusive can't be given since then it won't be exclusive anymore.
Currently when markNetxRunning can not get a lock it simply continues. This is something that needs to be changed.
Instead of introducing a new lock, we can just make the plugin wait for clearing cache to finish if cache clearing is in progress by using lock() instead of tryLock().
 Similar to your example we can do lock on KEY_USER_NETX_RUNNING_FILE, instead of try lock. With the additional change to okToClearCache by removing the finally block for closing the stream.

>From debugging, we can be sure that we will mark netx as running before we proceed to download/initialize the resources. So we won't need to worry about deleting right after we download or in process of downloading the jars.

In the event that we can only get exclusive locks, this means we can only have 1 plugin, or 1 javaws running at a time. we can however still have many applets running from the same jvm.
This can be solved by having essentially a semaphore kind of structure. (Don't like this...JVM can be killed via kill -9 and it won't decrement the count).
Another way to do this is give them all their own unique cache directory, no more worries...But this defeats the purpose of a cache..
Any other suggestions?

Regards,
  Andrew

> 
> if (!tryLock(CL)) fail_clear_cache
> if (!tryLock(KEY_USER_NETX_RUNNING_FILE)) // we already do this
> release(CL)
> fail_clear_cache
> 
> clear the cache
> release(CL);
> 
> 
> and in markNetxRunning we would do:
> 
> tryLock(KEY_USER_NETX_RUNNING_FILE) // already do this
> lock(CL) // this blocks
> release(CL)
> 
> 
> Regards,
> Denis.
> 
> ----- Original Message -----
> > > > Hi,
> > > >
> > > > This is a fix to an issue where you can use javaws -Xclearcache
> > > > to
> > > > remove the cache directory while an applet is running through
> > > > the
> > > > browser. This causes the applets to fail to load and other
> > > > disastrous
> > > > things to happen.
> > > >
> > > > To fix this I moved Launcher.markNetxRunning() and
> > > > Launcher.markNetxStopped() to JNLPRuntime class. I believe these
> > > > are
> > > > closely related to the "runtime" of the plugin and/or javaws, so
> > > > it
> > > > makes sense to put them there.
> > > >
> > > > We call JNLPRuntime.markNetxRunning() before we launch
> > > > something,
> > > > (application or applet). Adding a check to markNetxRunning() so
> > > > that
> > > > if
> > > > we have already been marks, just continue. Since we are marking
> > > > it
> > > > as
> > > > running, we also want to mark it as not running when we shutdown
> > > > the
> > > > JVM. The call to adding the shutdown hook for markNetxStopped()
> > > > is
> > > > moved
> > > > to the end of the markNetxRunning method.
> > > >
> > > > 2011-03-14 Andrew Su <asu at redhat.com>
> > > >
> > > > * netx/net/sourceforge/jnlp/Launcher.java:
> > > > (launch): Mark NetX as running before launching apps.
> > > > (launchApplication): Removed call to markNetxRunning().
> > > > (markNetxRunning): Removed method.
> > > > (markNetxStopped): Removed method.
> > > > * netx/net/sourceforge/jnlp/runtime/JNLPRuntime.java:
> > > > (fileLock): New private static field.
> > > > (markNetxRunning): New method to indicate NetX is running.
> > > > (markNetxStopped): New method to indicate NetX has stopped.
> > > >
> > > > Minor Concerns:
> 
> 
> >
> > Patch updated to work with head.