changeset in /hg/icedtea6: - Fix race conditions related to mult...
Deepak Bhole
dbhole at redhat.com
Wed Jun 17 11:44:37 PDT 2009
changeset d044933640b4 in /hg/icedtea6
details: http://icedtea.classpath.org/hg/icedtea6?cmd=changeset;node=d044933640b4
description:
- Fix race conditions related to multi-stage initialization/destruction
- Account for port# when implementing security policy based on address
- Reduce sleep time during initialization check iterations
diffstat:
4 files changed, 168 insertions(+), 32 deletions(-)
ChangeLog | 9 +
IcedTeaPlugin.cc | 22 ++
plugin/icedtea/sun/applet/PluginAppletViewer.java | 167 +++++++++++++++++----
rt/net/sourceforge/jnlp/NetxPanel.java | 2
diffs (359 lines):
diff -r 4a617634d81c -r d044933640b4 ChangeLog
--- a/ChangeLog Wed Jun 17 14:37:31 2009 +0100
+++ b/ChangeLog Wed Jun 17 14:47:36 2009 -0400
@@ -1,3 +1,12 @@ 2009-06-17 Gary Benson <gbenson at redhat
+2009-06-17 Deepak Bhole <dbhole at redhat.com>
+
+ * IcedTeaPlugin.cc: Fix race condition that led to segfault.
+ * plugin/icedtea/sun/applet/PluginAppletViewer.java: Fix a host of race
+ conditions brought about by the multi stage asynchronous initialization
+ design.
+ * rt/net/sourceforge/jnlp/NetxPanel.java: Account for case where handler
+ may be null due to an error above.
+
2009-06-17 Gary Benson <gbenson at redhat.com>
* ports/hotspot/src/share/vm/shark/sharkCacheDecache.hpp
diff -r 4a617634d81c -r d044933640b4 IcedTeaPlugin.cc
--- a/IcedTeaPlugin.cc Wed Jun 17 14:37:31 2009 +0100
+++ b/IcedTeaPlugin.cc Wed Jun 17 14:47:36 2009 -0400
@@ -1065,6 +1065,7 @@ private:
gpointer window_handle;
guint32 window_width;
guint32 window_height;
+ PRBool is_active;
// FIXME: nsCOMPtr.
IcedTeaPluginFactory* factory;
PRUint32 instance_identifier;
@@ -2447,6 +2448,7 @@ IcedTeaPluginInstance::Destroy ()
nsCString destroyMessage (instanceIdentifierPrefix);
destroyMessage += "destroy";
factory->SendMessageToAppletViewer (destroyMessage);
+ is_active = PR_FALSE;
return NS_OK;
}
@@ -2475,7 +2477,9 @@ IcedTeaPluginInstance::SetWindow (nsPlug
long startTime = get_time_in_s();
PRBool timedOut = PR_FALSE;
- while (initialized == PR_FALSE && this->fatalErrorOccurred == PR_FALSE)
+ while (initialized == PR_FALSE &&
+ this->fatalErrorOccurred == PR_FALSE &&
+ this->is_active == PR_FALSE)
{
PROCESS_PENDING_EVENTS;
@@ -2638,7 +2642,9 @@ IcedTeaPluginInstance::GetJavaObject (jo
long startTime = get_time_in_s();
PRBool timedOut = PR_FALSE;
- while (initialized == PR_FALSE && this->fatalErrorOccurred == PR_FALSE)
+ while (initialized == PR_FALSE &&
+ this->fatalErrorOccurred == PR_FALSE &&
+ this->is_active == PR_FALSE)
{
PROCESS_PENDING_EVENTS;
@@ -4115,6 +4121,7 @@ IcedTeaPluginInstance::IcedTeaPluginInst
liveconnect_window (0),
initialized(PR_FALSE),
fatalErrorOccurred(PR_FALSE),
+ is_active(PR_TRUE),
instanceIdentifierPrefix ("")
{
PLUGIN_TRACE_INSTANCE ();
@@ -4134,6 +4141,7 @@ IcedTeaPluginInstance::GetWindow ()
nsresult result;
PLUGIN_DEBUG_1ARG ("HERE 22: %d\n", liveconnect_window);
+
// principalsArray, numPrincipals and securitySupports
// are ignored by GetWindow. See:
//
@@ -4144,6 +4152,16 @@ IcedTeaPluginInstance::GetWindow ()
if (factory->proxyEnv != NULL)
{
PLUGIN_DEBUG_2ARG ("HERE 23: %d, %p\n", liveconnect_window, current_thread ());
+
+ // there is a bad race condition here where if the instance is active,
+ // this code remains active after destruction.. so double check
+ if (is_active != PR_TRUE)
+ {
+ PLUGIN_DEBUG_1ARG("Plugin %d is no longer active. Bypassing \
+ GetWindow request.\n", instance_identifier);
+ return;
+ }
+
result = factory->liveconnect->GetWindow(factory->proxyEnv,
this,
NULL, 0, NULL,
diff -r 4a617634d81c -r d044933640b4 plugin/icedtea/sun/applet/PluginAppletViewer.java
--- a/plugin/icedtea/sun/applet/PluginAppletViewer.java Wed Jun 17 14:37:31 2009 +0100
+++ b/plugin/icedtea/sun/applet/PluginAppletViewer.java Wed Jun 17 14:47:36 2009 -0400
@@ -145,6 +145,8 @@ import com.sun.jndi.toolkit.url.UrlUtil;
*/
private static String defaultSaveFile = "Applet.ser";
+ private static enum PAV_INIT_STATUS {PRE_INIT, ACTIVE, INACTIVE};
+
/**
* The panel in which the applet is being displayed.
*/
@@ -168,17 +170,23 @@ import com.sun.jndi.toolkit.url.UrlUtil;
int identifier;
- private static HashMap<Integer, PluginParseRequest> requests = new HashMap();
+ private static HashMap<Integer, PluginParseRequest> requests =
+ new HashMap();
// Instance identifier -> PluginAppletViewer object.
- private static HashMap<Integer, PluginAppletViewer> applets = new HashMap();
+ private static HashMap<Integer, PluginAppletViewer> applets =
+ new HashMap();
private static PluginStreamHandler streamhandler;
private static PluginCallRequestFactory requestFactory;
- private static HashMap<Integer, String> siteCookies = new HashMap<Integer,String>();
-
+ private static HashMap<Integer, String> siteCookies =
+ new HashMap<Integer,String>();
+
+ private static HashMap<Integer, PAV_INIT_STATUS> status =
+ new HashMap<Integer,PAV_INIT_STATUS>();
+
private double proposedHeightFactor;
private double proposedWidthFactor;
@@ -312,7 +320,7 @@ import com.sun.jndi.toolkit.url.UrlUtil;
Applet a;
while ((a = panel.getApplet()) == null && ((NetxPanel) panel).isAlive()) {
try {
- Thread.sleep(2000);
+ Thread.sleep(1000);
PluginDebug.debug("Waiting for applet to initialize... ");
} catch (InterruptedException ie) {
ie.printStackTrace();
@@ -328,7 +336,8 @@ import com.sun.jndi.toolkit.url.UrlUtil;
PluginDebug.debug("Applet initialized");
// Applet initialized. Find out it's classloader and add it to the list
- String codeBase = doc.getProtocol() + "://" + doc.getHost();
+ String portComponent = doc.getPort() != -1 ? ":" + doc.getPort() : "";
+ String codeBase = doc.getProtocol() + "://" + doc.getHost() + portComponent;
if (atts.get("codebase") != null) {
try {
@@ -373,6 +382,21 @@ import com.sun.jndi.toolkit.url.UrlUtil;
// may happen in independent threads
synchronized(requests) {
+
+ // Check if we should proceed with init
+ // (=> no if destroy was called after tag, but before
+ // handle)
+ if (status.containsKey(identifier) &&
+ status.get(identifier).equals(PAV_INIT_STATUS.INACTIVE)) {
+
+ PluginDebug.debug("Inactive flag set. Refusing to initialize instance " + identifier);
+ requests.remove(identifier);
+ return;
+
+ }
+
+ status.put(identifier, PAV_INIT_STATUS.PRE_INIT);
+
PluginParseRequest request = requests.get(identifier);
if (request == null) {
request = new PluginParseRequest();
@@ -393,6 +417,17 @@ import com.sun.jndi.toolkit.url.UrlUtil;
new StringReader(request.tag),
new URL(request.documentbase));
requests.remove(identifier);
+
+ // Panel initialization cannot be aborted mid-way.
+ // Once it is initialized, double check to see if this
+ // panel needs to stay around..
+ if (status.get(identifier).equals(PAV_INIT_STATUS.INACTIVE)) {
+ PluginDebug.debug("Inactive flag set. Destroying applet instance " + identifier);
+ applets.get(identifier).handleMessage(-1, "destroy");
+ } else {
+ status.put(identifier, PAV_INIT_STATUS.ACTIVE);
+ }
+
} else {
PluginDebug.debug ("REQUEST HANDLE NOT SET: " + request.handle + ". BYPASSING");
}
@@ -400,6 +435,21 @@ import com.sun.jndi.toolkit.url.UrlUtil;
} else if (message.startsWith("handle")) {
synchronized(requests) {
+
+ // Check if we should proceed with init
+ // (=> no if destroy was called after handle, but before
+ // tag)
+ if (status.containsKey(identifier) &&
+ status.get(identifier).equals(PAV_INIT_STATUS.INACTIVE)) {
+
+ PluginDebug.debug("Inactive flag set. Refusing to initialize instance " + identifier);
+ requests.remove(identifier);
+ return;
+
+ }
+
+ status.put(identifier, PAV_INIT_STATUS.PRE_INIT);
+
PluginParseRequest request = requests.get(identifier);
if (request == null) {
request = new PluginParseRequest();
@@ -418,6 +468,17 @@ import com.sun.jndi.toolkit.url.UrlUtil;
requests.remove(identifier);
PluginDebug.debug ("REQUEST HANDLE, DONE PARSING " +
Thread.currentThread());
+
+ // Panel initialization cannot be aborted mid-way.
+ // Once it is initialized, double check to see if this
+ // panel needs to stay around..
+ if (status.get(identifier).equals(PAV_INIT_STATUS.INACTIVE)) {
+ PluginDebug.debug("Inactive flag set. Destroying applet instance " + identifier);
+ applets.get(identifier).handleMessage(-1, "destroy");
+ } else {
+ status.put(identifier, PAV_INIT_STATUS.ACTIVE);
+ }
+
} else {
PluginDebug.debug ("REQUEST TAG NOT SET: " + request.tag + ". BYPASSING");
}
@@ -433,12 +494,57 @@ import com.sun.jndi.toolkit.url.UrlUtil;
// Always set the cookie -- even if it is null
siteCookies.put(identifier, cookieStr);
} else {
- PluginDebug.debug ("HANDLING MESSAGE " + message + " instance " + identifier + " " + Thread.currentThread());
+ PluginDebug.debug ("Handling message: " + message + " instance " + identifier + " " + Thread.currentThread());
+
+ // Destroy may be called while initialization is still going
+ // on. We therefore special case it.
+ if (!applets.containsKey(identifier) && message.equals("destroy")) {
+
+ // Set the status to inactive right away. Doesn't matter if it
+ // gets clobbered during init. due to a race. That is what the
+ // double check below is for.
+ PluginDebug.debug("Destroy called during initialization. Delaying destruction.");
+ status.put(identifier, PAV_INIT_STATUS.INACTIVE);
+
+ // We have set the flags. We now lock what stage 1 and 2
+ // lock on, and force a synchronous status check+action.
+ synchronized (requests) {
+ // re-check (inside lock) if the applet is
+ // initialized at this point.
+ if (applets.containsKey(identifier)) {
+ PluginDebug.debug("Init done. destroying normally.");
+ applets.get(identifier).handleMessage(reference, message);
+ } else {
+ }
+ } // unlock
+
+ // we're done here
+ return;
+ }
+
+ // For messages other than destroy, wait till initialization finishes
+ while (!applets.containsKey(identifier) &&
+ (
+ !status.containsKey(identifier) ||
+ status.get(identifier).equals(PAV_INIT_STATUS.PRE_INIT)
+ )
+ );
+
+ // don't bother processing further for inactive applets
+ if (status.get(identifier).equals(PAV_INIT_STATUS.INACTIVE))
+ return;
+
applets.get(identifier).handleMessage(reference, message);
}
} catch (Exception e) {
- throw new RuntimeException("Failed to handle message: " + message + " " +
- Thread.currentThread(), e);
+
+ // If an exception happened during pre-init, we need to update status
+ if (status.get(identifier).equals(PAV_INIT_STATUS.PRE_INIT))
+ status.put(identifier, PAV_INIT_STATUS.INACTIVE);
+
+ throw new RuntimeException("Failed to handle message: " +
+ message + " for instance " + identifier + " " +
+ Thread.currentThread(), e);
}
}
@@ -487,6 +593,7 @@ import com.sun.jndi.toolkit.url.UrlUtil;
}
} else if (message.startsWith("destroy")) {
dispose();
+ status.put(identifier, PAV_INIT_STATUS.INACTIVE);
} else if (message.startsWith("GetJavaObject")) {
// FIXME: how do we determine what security context this
// object should belong to?
@@ -1291,26 +1398,28 @@ import com.sun.jndi.toolkit.url.UrlUtil;
* the last applet.
*/
void appletClose() {
-
- // The caller thread is event dispatch thread, so
- // spawn a new thread to avoid blocking the event queue
- // when calling appletShutdown.
- //
- final AppletPanel p = panel;
-
- new Thread(new Runnable()
- {
- public void run()
- {
- appletShutdown(p);
- appletPanels.removeElement(p);
- dispose();
-
- if (countApplets() == 0) {
- appletSystemExit();
- }
- }
- }).start();
+
+ // The caller thread is event dispatch thread, so
+ // spawn a new thread to avoid blocking the event queue
+ // when calling appletShutdown.
+ //
+ final AppletPanel p = panel;
+
+ new Thread(new Runnable()
+ {
+ public void run()
+ {
+ appletShutdown(p);
+ appletPanels.removeElement(p);
+ dispose();
+
+ if (countApplets() == 0) {
+ appletSystemExit();
+ }
+ }
+ }).start();
+
+ status.put(identifier, PAV_INIT_STATUS.INACTIVE);
}
/**
diff -r 4a617634d81c -r d044933640b4 rt/net/sourceforge/jnlp/NetxPanel.java
--- a/rt/net/sourceforge/jnlp/NetxPanel.java Wed Jun 17 14:37:31 2009 +0100
+++ b/rt/net/sourceforge/jnlp/NetxPanel.java Wed Jun 17 14:47:36 2009 -0400
@@ -144,7 +144,7 @@ public class NetxPanel extends AppletVie
}
public boolean isAlive() {
- return handler.isAlive() && this.appletAlive;
+ return handler != null && handler.isAlive() && this.appletAlive;
}
}
More information about the distro-pkg-dev
mailing list