[rfc][icedtea-web] Ensure applet shutdown finishes without exceptions

Adam Domurad adomurad at redhat.com
Thu Jan 31 14:05:52 PST 2013


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.

Fixed in dont-interrupt-workers.patch:

3.) Applets were being interrupted while shutting down -- a major cause 
of spurious exceptions being printed out (causing even more noise in 
reproducer system). This interrupt facility was too heavy-weight for the 
desired task. Instead Java's builtin wait & notify facilities are now 
used to perform the same operation on a much finer scale.

** Note that I looked into the matter and saw that the method 
notifyWorkerIsFree did not actually perform the desired task and was 
removed as misleading. In fact the consumer thread spins around looking 
for free workers and did not wait for an interruption. This behaviour 
has not changed, and the method is not needed (nor was it needed how the 
code was before).

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

dont-interrupt-workers.patch changes:
2013-XX-XX  Adam Domurad  <adomurad at redhat.com>

     Don't interrupt worker/consumer threads (can prevent shutdown code 
from
     executing); instead use Object wait/notify methods.
     * plugin/icedteanp/java/sun/applet/PluginMessageConsumer.java
     (notifyHasWork): Replacement for thread interruption
     (waitForWork): Replacement for thread sleeping
     (run): Use waitForWork instead of Thread.sleep
     (notifyWorkerIsFree): Removed -- misleading method.
     * netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java
     (message): Make volatile, as it should have always been.
     (notifyHasWork): Replacement for thread interruption
     (waitForWork): Replacement for thread sleeping
     (run): Use waitForWork instead of Thread.sleep
     (getPermissions): avoid potential NPE if code source location is
     missing
     (free): Remove reference to notifyWorkerIsFree.


I am excited to see these changes get in. I will see soon if there's any 
effect on the amount of noise in the test system.

Happy hacking,
-Adam
-------------- next part --------------
A non-text attachment was scrubbed...
Name: npe-fix-and-wait-for-init.patch
Type: text/x-patch
Size: 1401 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20130131/d83de69a/npe-fix-and-wait-for-init.patch 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: dont-interrupt-workers.patch
Type: text/x-patch
Size: 4348 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20130131/d83de69a/dont-interrupt-workers.patch 


More information about the distro-pkg-dev mailing list