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

[Denis Lila]

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.

> > > - 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. 

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:

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.