RFC: Netx - implement -Xclearcache command line option

[Omair Majid]

On 10/06/2010 02:40 PM, Dr Andrew John Hughes wrote:
> On 12:04 Wed 06 Oct     , Omair Majid wrote:
>> 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.
>>
>
> I like netx_running best.  It's clear and obvious :-)
>
>>>
>>>> +            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().
>>
>
> Does it have to be called R and not 'translate' or something more readable?
>

I am sure we can change it to something more sensible. I was just 
sticking with what the rest of Netx uses (there is already a R function 
in the same class). Still, I would like to handle that in a separate 
patch if it is ok.

BTW, I am sure you know that a lot of programs use _ as the translate 
function. Is _ a valid java identifier? ;)

>>> 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.
>>
>
> That sounds sensible.  I'm just always wary of catch blocks.  I've seen them
> used all too often to mask exceptions instead of handling them (including in
> example code used by lecturers!)
>

Heh. That's true. I think checked exceptions even (sort of) encourage 
this. I am afraid of swallowing exceptions too (or worse, handling them 
at the wrong level), but in this particular instance it makes sense to 
do just that.

Thanks for looking over the changes.

Cheers,
Omair