RFC: Netx - implement -Xclearcache command line option

[Omair Majid]

On 10/05/2010 07:19 PM, Dr Andrew John Hughes wrote:
> On 14:45 Tue 05 Oct     , Omair Majid wrote:
>> On 10/04/2010 03:17 PM, Omair Majid wrote:
>>> On 09/30/2010 05:27 PM, Deepak Bhole wrote:
>>>> * Omair Majid<omajid at redhat.com>  [2010-09-30 16:50]:
>>>>> On 09/30/2010 04:32 PM, Deepak Bhole wrote:
>>>>>> * Omair Majid<omajid at redhat.com>  [2010-09-30 16:10]:
>>>>>>> Hi,
>>>>>>>
>>>>>>> I posted this patch way back in 2009, but it looks like it got
>>>>>>> lost/ignored.
>>>>>>>
>>>>>
>>>>> Ok, turns out it was my fault: just noticed
>>>>> http://mail.openjdk.java.net/pipermail/distro-pkg-dev/2009-August/006666.html.
>>>>>
>>>>> Somehow it slipped off my radar.
>>>>>
>>>>
>>>> Doh. Glad it was re-visited. I doubt the issue will be encountered, but
>>>> we might as well fix it instead of waiting for a bug report.
>>>>
>>>>>>> On 07/30/2009 05:18 PM, Omair Majid wrote:
>>>>>>>> The attached patch adds the option -Xclearcache to Netx.
>>>>>>>>
>>>>>>>
>>>>>>> Which (and I neglected to explain this last time) makes javaws clean
>>>>>>> out its cache by deleting the contents of ~/.netx/cache/
>>>>>>>
>>>>>>
>>>>>> Hmm, what happens if I have another netx app running and javaws is
>>>>>> called
>>>>>> with -Xclearcache? AFAIK the code has no provisioning to detect if a
>>>>>> cache file has been deleted, and to re-download it... does it?
>>>>>>
>>>>>
>>>>> As far as I can tell, very bad things (not eating babies kind of
>>>>> bad, but still...). Depending on the exact timing either the VM will
>>>>> crash or a java Exception might be thrown. If the JVM has opened the
>>>>> (missing) jar earlier and and wants to open it again to load
>>>>> additional classes the VM will most probably crash. Otherwise javaws
>>>>> simply wont be able to find the needed jar and will fail to start.
>>>>>
>>>>> I suppose I could make some sort of global application lock to stop
>>>>> any other javaws instances from starting while -Xclearcache is
>>>>> running and to make sure -Xclearcache does not start if any other
>>>>> javaws applications are running.
>>>>>
>>>>
>>>> It would be hard to set up a locking system that can handle odd cases
>>>> like crashed application, multiple instances of same app., etc.
>>>>
>>>
>>> Yeah, you are right. I thought about it for a bit, and I dont see a
>>> simple way to make it work using locks.
>>>
>>>> Easiest way I think is to execute jps -l and see if there is more than
>>>> one instance of net.sourceforge.jnlp.runtime.Boot. If there is, refuse
>>>> to run with -Xclearcache. I am not as worried about the 'don't start
>>>> while -Xclearcache is running' case, as there is a far smaller window
>>>> where things can go wrong. Plus if they do, it will be right away (as
>>>> opposed to an already started app that has modified data, crashing).
>>>>
>>>
>>> That's a great idea! Unfortunately I dont think we can rely on jps. jps
>>> is part of the JDK as opposed to the JRE - we can not be sure that users
>>> have it on their machine. I tried looking into the sources for jps to
>>> find out how it works. The JVM creates /tmp/hsperfdata_$USER/$PID files
>>> which contain the information that jps can parse and display. The files
>>> are in a binary format so parsing it is not trivial (and the file format
>>> is not guaranteed to be final). I am going to dig around a bit to see if
>>> there is any other simple solution to this
>>>
>>
>> I am attaching the updated patch. This one uses an alternate
>> implementation based on file locks (exclusive and shared). A normal
>> javaws process acquires a shared lock on /tmp/$USER/netx/instance during
>> startup (when an instance of Launcher is created), and releases the lock
>> when the JVM shutsdown. Multiple javaws processes can share the lock and
>> run simultaneously.
>>
>> The javaws process that is trying to clear the cache checks for the file
>> and tries to acquire an exclusive lock. If it succeeds, it goes ahead
>> and clears the cache; otherwise it prints out an error to the user and
>> exits.
>>
>> Thoughts?
>>
>
> Sounds good to me.  I was actually going to suggest this before Deepak weighed
> in with the jps option.  Not knowing what jps was, I didn't no any pros or cons
> as to using it.
>

Updated patch attached. Also fixes another problem - the previous patch 
was creating lock files for applets too :/.

> Could you not just reuse LOCKS_DIR for this rather than introducing something
> new?

Ah, good point. Switched to $LOCKS_DIR/netx_running.

> Also your mail says 'instance' but the code says 'instances' :-)

I cant come up with a name I am happy with... something that indicates 
the purpose of those files clearly. NETX_RUNNING_FILE and 
$/LOCKS/netx_running is what I am using now.

>
>> +            FileChannel channel = fileLock.channel();
>> +            fileLock.release();
>> +            channel.close();
>
> could be just:
>
> fileLock.release();
> fileLock.channel().close();
>

Nice catch. Fixed.

> Why do we have a method called just 'R' in CacheUtil?
>

R is the translation function. It is supposed to localize strings. A 
number of classes in netx declare R, which is a shortcut to 
JNLPRuntime.getMessage().

> Should we really be catching IOExceptions rather than allowing them to propogate?
>

Catching IOExceptions is just ignoring the errors. So the question is do 
we want to ignore the errors and continue on or should we stop if we hit 
any errors. Since the lock file is meant to stop a race condition 
(between deleting jars and using them) which is not likely to happen, I 
think an error in lock file creation should be ignored. If for some 
reason a lock file can not be created, Netx should not abort running the 
application. If you dont think this makes sense, please let me know and 
I will change it.

Thanks for the feedback!

Cheers,
Omair
-------------- next part --------------
A non-text attachment was scrubbed...
Name: icedtea6-jnlp-clearcache-04.patch
Type: text/x-patch
Size: 12747 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20101006/541ba48e/icedtea6-jnlp-clearcache-04.patch