[rfc][icedtea-web] Ensure applet shutdown finishes without exceptions - 1 - npe
Adam Domurad
adomurad at redhat.com
Tue Feb 5 08:46:40 PST 2013
On 02/01/2013 04:18 AM, Jiri Vanek wrote:
> On 01/31/2013 11:05 PM, Adam Domurad wrote:
>> Hi all. Another round of what I think could be important stability
>> fixes for icedtea-web.
>>
>> I noticed while doing my unsigned-applet-confirmation patch that
>> JNLPClassLoader instances did not always reach reference-count == 0
>> when all their instances should have been destroyed. The first part
>> of that was the applet synchronization issue, that I have posted a
>> patch for.
>>
>> 3 additional problems fixed here:
>>
>> Fixed in npe-fix-and-wait-for-init.patch:
>> 1.) There was a sometimes-occurring NPE in
>> JNLPClassLoader#getPermisions, when a CodeSource did not have an
>> associated location when an applet was destroyed. This could cause
>> the destroyApplet code to not finish. This was fixed with a simple
>> null check guard. This fixes some spurious exceptions being printed
>> out during shutdown (causing some noise in reproducer system).
>>
>> 2.) Applets can actually be destroyed from one thread *in the middle
>> of initialization*, while somewhat hard to reproduce, spamming
>> refresh on a many-applet page inevitably caused it. Luckily a
>> mechanism was already in place for efficiently waiting for applets to
>> initialize.
>>
>>
>> npe-fix-and-wait-for-init.patch changes:
>> 2013-XX-XX Adam Domurad <adomurad at redhat.com>
>>
>> Ensure applet destruction cannot in the middle of initialization.
>> Prevent NPE that can sometimes occurs (especially with this
>> change).
>> * netx/net/sourceforge/jnlp/NetxPanel.java
>> (destroyApplet): wait for applet initialization
>> * netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java
>> (getPermissions): avoid potential NPE if code source location is
>> missing
>>
>
>>
>>
>> npe-fix-and-wait-for-init.patch
>>
>>
>> diff --git a/netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java
>> b/netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java
>> --- a/netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java
>> +++ b/netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java
>> @@ -1144,7 +1144,7 @@ public class JNLPClassLoader extends URL
>> }
>>
>> // Class from host X should be allowed to connect to
>> host X
>> - if (cs.getLocation().getHost().length()> 0)
>> + if (cs.getLocation() != null&&
>> cs.getLocation().getHost().length()> 0)
>
> Here I'm curious, is NPE here because location was not yet
> initalised, have no chance to get in, its initialisation was
> corrupted, or is NPE here because its it wa forgotten to be assigned
> or its creation was somehow skipped?
>
> It it is first case then I'm for NPE guard.
> If it is the second then I'm against, because the consequence is
> healed and the cause will be rotten later. However still the NPE guard
> can be last and only hope.
I'll admit I don't know the exact situation that causes this, but it
only occurred during shutdown. Seeing as I can't reproduce it ATM I'm
fine having this or not, but I don't think it would hide any problems.
>
>> result.add(new
>> SocketPermission(cs.getLocation().getHost(),
>> "connect, accept"));
>>
>> diff --git a/plugin/icedteanp/java/sun/applet/PluginAppletViewer.java
>> b/plugin/icedteanp/java/sun/applet/PluginAppletViewer.java
>> --- a/plugin/icedteanp/java/sun/applet/PluginAppletViewer.java
>> +++ b/plugin/icedteanp/java/sun/applet/PluginAppletViewer.java
>> @@ -587,6 +587,10 @@ public class PluginAppletViewer extends
>>
>> private static synchronized void destroyApplet(int identifier) {
>>
>> + // We should not try to destroy an applet during
>> + // initialization. It may cause an inconsistent state.
>> + waitForAppletInit( applets.get(identifier).panel );
>
> Ok, this have sense.
>> +
>> PluginDebug.debug("DestroyApplet called for ", identifier);
>>
>> PAV_INIT_STATUS prev = updateStatus(identifier,
>> PAV_INIT_STATUS.DESTROYED);
>>
-Adam
More information about the distro-pkg-dev
mailing list