/hg/icedtea6: PR556: Re-implement applet initialization to be se...
dbhole at icedtea.classpath.org
dbhole at icedtea.classpath.org
Thu Sep 16 10:03:56 PDT 2010
changeset c764b38139a5 in /hg/icedtea6
details: http://icedtea.classpath.org/hg/icedtea6?cmd=changeset;node=c764b38139a5
author: Deepak Bhole <dbhole at redhat.com>
date: Thu Sep 16 13:03:57 2010 -0400
PR556: Re-implement applet initialization to be serialialized, to
fix current and future possible race conditions.
diffstat:
8 files changed, 232 insertions(+), 315 deletions(-)
ChangeLog | 41 +
NEWS | 2
plugin/icedteanp/IcedTeaJavaRequestProcessor.h | 2
plugin/icedteanp/IcedTeaNPPlugin.cc | 67 +
plugin/icedteanp/IcedTeaNPPlugin.h | 4
plugin/icedteanp/IcedTeaPluginUtils.cc | 8
plugin/icedteanp/java/sun/applet/PluginAppletViewer.java | 378 ++++-------
plugin/icedteanp/java/sun/applet/PluginMessageConsumer.java | 45 -
diffs (truncated from 967 to 500 lines):
diff -r 1c5acca8abfd -r c764b38139a5 ChangeLog
--- a/ChangeLog Mon Sep 13 10:21:58 2010 -0400
+++ b/ChangeLog Thu Sep 16 13:03:57 2010 -0400
@@ -1,3 +1,44 @@ 2010-09-13 Omair Majid <omajid at redhat.
+2010-09-15 Deepak Bhole <dbhole at redhat.com>
+
+ PR556: Re-implement applet initialization to be serialialized, to fix
+ current and future possible race conditions.
+ * plugin/icedteanp/IcedTeaJavaRequestProcessor.h: Updated timeout to 3
+ minutes.
+ * plugin/icedteanp/IcedTeaNPPlugin.cc
+ (ITNP_New): Remove the tag_message string, don't send any data to Java
+ side and instead, populate the new ITNPPluginData.applet_tag field with
+ the tag info. Do cleanup accordingly.
+ (ITNP_SetWindow): Send all (handle, size and tag) initialization
+ information to java in a single message. Cleanup accordingly for newly
+ added variables.
+ * plugin/icedteanp/IcedTeaPluginUtils.cc
+ (getReference): Use a negative decreasing reference count to prevent
+ clashes with references for Java-side requests.
+ * plugin/icedteanp/IcedTeaNPPlugin.h: Rename the instance_string field of
+ ITNPPluginData to instance_id, and add an applet_tag field.
+ * plugin/icedteanp/java/sun/applet/PluginAppletViewer.java
+ (PluginAppletPanelFactory.createPanel): Remove code that tried to account
+ for percent heights and widths. This is done by the browser now.
+ (reFrame): Same.
+ (PluginAppletViewer): Update constructor to not take height and width
+ factor information.
+ (handleMessage): Update function for new protocol that now passes all
+ information relevant to initialization in a single message.
+ (skipSpace): Accept a new int array that contains the character at the
+ current position in the stream. Use the value accordingly.
+ (scanIdentifier): Same.
+ (skipComment): Same.
+ (scanTag): Same.
+ (parse): Updated all function signatures to accept width and height info.
+ Initialize an int array (for fake pass-by-reference) that contains
+ position of current character in the stream. Inject width and height
+ information into the atts table.
+ * plugin/icedteanp/java/sun/applet/PluginMessageConsumer.java: Updated
+ thread count needed for initialization.
+ (okayToProcess): Remove function that is no longer needed with the
+ serialized initialization code.
+ (ConsumerThread.run): Remove call to okayToProcess().
+
2010-09-13 Omair Majid <omajid at redhat.com>
Add a new man page for netx's javaws.
diff -r 1c5acca8abfd -r c764b38139a5 NEWS
--- a/NEWS Mon Sep 13 10:21:58 2010 -0400
+++ b/NEWS Thu Sep 16 13:03:57 2010 -0400
@@ -16,6 +16,8 @@ New in release 1.10 (2010-XX-XX):
- S6438179: XToolkit.isTraySupported() result has nothing to do with the system tray
* Netx
- A new man page for javaws.
+* Plugin
+ - PR556: Applet initialization code is prone to race conditions
New in release 1.9 (2010-09-07):
diff -r 1c5acca8abfd -r c764b38139a5 plugin/icedteanp/IcedTeaJavaRequestProcessor.h
--- a/plugin/icedteanp/IcedTeaJavaRequestProcessor.h Mon Sep 13 10:21:58 2010 -0400
+++ b/plugin/icedteanp/IcedTeaJavaRequestProcessor.h Thu Sep 16 13:03:57 2010 -0400
@@ -46,7 +46,7 @@ exception statement from your version. *
#include "IcedTeaNPPlugin.h"
#include "IcedTeaPluginUtils.h"
-#define REQUESTTIMEOUT 120
+#define REQUESTTIMEOUT 180
/*
* This struct holds data specific to a Java operation requested by the plugin
diff -r 1c5acca8abfd -r c764b38139a5 plugin/icedteanp/IcedTeaNPPlugin.cc
--- a/plugin/icedteanp/IcedTeaNPPlugin.cc Mon Sep 13 10:21:58 2010 -0400
+++ b/plugin/icedteanp/IcedTeaNPPlugin.cc Thu Sep 16 13:03:57 2010 -0400
@@ -249,7 +249,7 @@ pthread_cond_t cond_message_available =
// Creates a new icedtea np plugin instance. This function creates a
// ITNPPluginData* and stores it in instance->pdata. The following
-// ITNPPluginData fiels are initialized: instance_string, in_pipe_name,
+// ITNPPluginData fiels are initialized: instance_id, in_pipe_name,
// in_from_appletviewer, in_watch_source, out_pipe_name,
// out_to_appletviewer, out_watch_source, appletviewer_mutex, owner,
// appletviewer_alive. In addition two pipe files are created. All
@@ -284,7 +284,6 @@ ITNP_New (NPMIMEType pluginType, NPP ins
gchar* documentbase = NULL;
gchar* read_message = NULL;
gchar* applet_tag = NULL;
- gchar* tag_message = NULL;
gchar* cookie_info = NULL;
NPObject* npPluginObj = NULL;
@@ -308,17 +307,17 @@ ITNP_New (NPMIMEType pluginType, NPP ins
// start the jvm if needed
start_jvm_if_needed();
- // Initialize data->instance_string.
+ // Initialize data->instance_id.
//
- // instance_string should be unique for this process so we use a
+ // instance_id should be unique for this process so we use a
// combination of getpid and plugin_instance_counter.
//
// Critical region. Reference and increment plugin_instance_counter
// global.
g_mutex_lock (plugin_instance_mutex);
- // data->instance_string
- data->instance_string = g_strdup_printf ("%d",
+ // data->instance_id
+ data->instance_id = g_strdup_printf ("%d",
instance_counter);
g_mutex_unlock (plugin_instance_mutex);
@@ -335,11 +334,8 @@ ITNP_New (NPMIMEType pluginType, NPP ins
// Send applet tag message to appletviewer.
applet_tag = plugin_create_applet_tag (argc, argn, argv);
- tag_message = (gchar*) malloc(strlen(applet_tag)*sizeof(gchar) + strlen(documentbase)*sizeof(gchar) + 32);
- g_sprintf(tag_message, "instance %d tag %s %s", instance_counter, documentbase, applet_tag);
-
- //plugin_send_message_to_appletviewer (data, data->instance_string);
- plugin_send_message_to_appletviewer (tag_message);
+ data->applet_tag = (gchar*) malloc(strlen(applet_tag)*sizeof(gchar) + strlen(documentbase)*sizeof(gchar) + 32);
+ g_sprintf(data->applet_tag, "tag %s %s", documentbase, applet_tag);
data->is_applet_instance = true;
}
@@ -371,8 +367,12 @@ ITNP_New (NPMIMEType pluginType, NPP ins
data->appletviewer_mutex = NULL;
// cleanup_instance_string:
- g_free (data->instance_string);
- data->instance_string = NULL;
+ g_free (data->instance_id);
+ data->instance_id = NULL;
+
+ // cleanup applet tag:
+ g_free (data->applet_tag);
+ data->applet_tag = NULL;
// cleanup_data:
// Eliminate back-pointer to plugin instance.
@@ -385,8 +385,6 @@ ITNP_New (NPMIMEType pluginType, NPP ins
instance->pdata = NULL;
cleanup_done:
- g_free (tag_message);
- tag_message = NULL;
g_free (applet_tag);
applet_tag = NULL;
g_free (read_message);
@@ -750,28 +748,33 @@ ITNP_SetWindow (NPP instance, NPWindow*
}
else
{
+ // Else this is initialization
PLUGIN_DEBUG ("ITNP_SetWindow: setting window.\n");
// Critical region. Send messages to appletviewer.
g_mutex_lock (data->appletviewer_mutex);
- gchar *window_message = g_strdup_printf ("instance %d handle %ld",
- id, (gulong) window->window);
- plugin_send_message_to_appletviewer (window_message);
- g_free (window_message);
+ // Store the window handle and dimensions
+ data->window_handle = window->window;
+ data->window_width = window->width;
+ data->window_height = window->height;
- window_message = g_strdup_printf ("instance %d width %d height %d",
- id,
- window->width,
- window->height);
- plugin_send_message_to_appletviewer (window_message);
- g_free (window_message);
- window_message = NULL;
+ // Now we have everything. Send this data to the Java side
+
+ gchar* instance_msg = g_strdup_printf ("instance %s handle %ld width %d height %d %s",
+ data->instance_id,
+ (gulong) data->window_handle,
+ data->window_width,
+ data->window_height,
+ data->applet_tag);
+
+ plugin_send_message_to_appletviewer (instance_msg);
+
+ g_free(instance_msg);
+ instance_msg = NULL;
g_mutex_unlock (data->appletviewer_mutex);
- // Store the window handle.
- data->window_handle = window->window;
}
PLUGIN_DEBUG ("ITNP_SetWindow return\n");
@@ -1921,8 +1924,12 @@ plugin_data_destroy (NPP instance)
tofree->appletviewer_mutex = NULL;
// cleanup_instance_string:
- g_free (tofree->instance_string);
- tofree->instance_string = NULL;
+ g_free (tofree->instance_id);
+ tofree->instance_id = NULL;
+
+ // cleanup applet tag
+ g_free (tofree->applet_tag);
+ tofree->applet_tag = NULL;
g_free(tofree->source);
tofree->source = NULL;
diff -r 1c5acca8abfd -r c764b38139a5 plugin/icedteanp/IcedTeaNPPlugin.h
--- a/plugin/icedteanp/IcedTeaNPPlugin.h Mon Sep 13 10:21:58 2010 -0400
+++ b/plugin/icedteanp/IcedTeaNPPlugin.h Thu Sep 16 13:03:57 2010 -0400
@@ -68,7 +68,9 @@ struct ITNPPluginData
struct ITNPPluginData
{
// A unique identifier for this plugin window.
- gchar* instance_string;
+ gchar* instance_id;
+ // The applet tag sent to Java side
+ gchar* applet_tag;
// Mutex to protect appletviewer_alive.
GMutex* appletviewer_mutex;
// Back-pointer to the plugin instance to which this data belongs.
diff -r 1c5acca8abfd -r c764b38139a5 plugin/icedteanp/IcedTeaPluginUtils.cc
--- a/plugin/icedteanp/IcedTeaPluginUtils.cc Mon Sep 13 10:21:58 2010 -0400
+++ b/plugin/icedteanp/IcedTeaPluginUtils.cc Thu Sep 16 13:03:57 2010 -0400
@@ -49,7 +49,7 @@ exception statement from your version. *
************************************************/
// Initialize static variables
-int IcedTeaPluginUtilities::reference = 0;
+int IcedTeaPluginUtilities::reference = -1;
pthread_mutex_t IcedTeaPluginUtilities::reference_mutex = PTHREAD_MUTEX_INITIALIZER;
std::map<void*, NPP>* IcedTeaPluginUtilities::instance_map = new std::map<void*, NPP>();
std::map<std::string, NPObject*>* IcedTeaPluginUtilities::object_map = new std::map<std::string, NPObject*>();
@@ -224,11 +224,11 @@ IcedTeaPluginUtilities::getReference()
pthread_mutex_lock(&reference_mutex);
// If we are nearing the max, reset
- if (reference > 0x7FFFFFFF - 10) {
- reference = 0;
+ if (reference < -0x7FFFFFFF + 10) {
+ reference = -1;
}
- reference++;
+ reference--;
pthread_mutex_unlock(&reference_mutex);
return reference;
diff -r 1c5acca8abfd -r c764b38139a5 plugin/icedteanp/java/sun/applet/PluginAppletViewer.java
--- a/plugin/icedteanp/java/sun/applet/PluginAppletViewer.java Mon Sep 13 10:21:58 2010 -0400
+++ b/plugin/icedteanp/java/sun/applet/PluginAppletViewer.java Thu Sep 16 13:03:57 2010 -0400
@@ -137,21 +137,10 @@ import com.sun.jndi.toolkit.url.UrlUtil;
}
});
- double heightFactor = 1.0;
- double widthFactor = 1.0;
-
- if (atts.get("heightPercentage") != null) {
- heightFactor = (Integer) atts.get("heightPercentage")/100.0;
- }
-
- if (atts.get("widthPercentage") != null) {
- widthFactor = (Integer) atts.get("widthPercentage")/100.0;
- }
// put inside initial 0 handle frame
- PluginAppletViewer.reFrame(null, identifier, System.out,
- heightFactor, widthFactor, 0, panel);
+ PluginAppletViewer.reFrame(null, identifier, System.out, 0, panel);
panel.init();
@@ -362,14 +351,12 @@ import com.sun.jndi.toolkit.url.UrlUtil;
private static HashMap<Integer, PAV_INIT_STATUS> status =
new HashMap<Integer,PAV_INIT_STATUS>();
- private double proposedHeightFactor;
- private double proposedWidthFactor;
private long handle = 0;
private WindowListener windowEventListener = null;
private AppletEventListener appletEventListener = null;
- public static final int APPLET_TIMEOUT = 60000;
+ public static final int APPLET_TIMEOUT = 180000;
private static Long requestIdentityCounter = 0L;
@@ -381,8 +368,7 @@ import com.sun.jndi.toolkit.url.UrlUtil;
public static void reFrame(PluginAppletViewer oldFrame,
int identifier, PrintStream statusMsgStream,
- double heightFactor, double widthFactor, long handle,
- AppletViewerPanel panel) {
+ long handle, AppletViewerPanel panel) {
PluginDebug.debug("Reframing " + panel);
@@ -393,7 +379,7 @@ import com.sun.jndi.toolkit.url.UrlUtil;
if (oldFrame != null && handle == oldFrame.handle)
return;
- PluginAppletViewer newFrame = new PluginAppletViewer(handle, identifier, statusMsgStream, heightFactor, widthFactor, panel);
+ PluginAppletViewer newFrame = new PluginAppletViewer(handle, identifier, statusMsgStream, panel);
if (oldFrame != null) {
applets.remove(oldFrame.identifier);
@@ -423,14 +409,12 @@ import com.sun.jndi.toolkit.url.UrlUtil;
* Create new plugin appletviewer frame
*/
private PluginAppletViewer(long handle, final int identifier,
- PrintStream statusMsgStream, double heightFactor,
- double widthFactor, AppletViewerPanel appletPanel) {
+ PrintStream statusMsgStream,
+ AppletViewerPanel appletPanel) {
super(handle, true);
this.statusMsgStream = statusMsgStream;
this.identifier = identifier;
- this.proposedHeightFactor = heightFactor;
- this.proposedWidthFactor = widthFactor;
this.panel = appletPanel;
if (!appletPanels.contains(panel))
@@ -525,87 +509,63 @@ import com.sun.jndi.toolkit.url.UrlUtil;
PluginDebug.debug("PAV handling: " + message);
try {
- if (message.startsWith("tag")) {
+ if (message.startsWith("handle")) {
- // tag and handle must both be set before parsing, so we need
- // synchronization here, as the setting of these variables
- // may happen in independent threads
+ // Extract the information from the message
+ String[] msgParts = new String[4];
+ for (int i=0; i < 3; i++) {
+ int spaceLocation = message.indexOf(' ');
+ int nextSpaceLocation = message.indexOf(' ', spaceLocation+1);
+ msgParts[i] = message.substring(spaceLocation + 1, nextSpaceLocation);
+ message = message.substring(nextSpaceLocation + 1);
+ }
- synchronized(requests) {
+ long handle = Long.parseLong(msgParts[0]);
+ String width = msgParts[1];
+ String height = msgParts[2];
- // 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)) {
+ int spaceLocation = message.indexOf(' ', "tag".length()+1);
+ String documentBase =
+ UrlUtil.decode(message.substring("tag".length() + 1, spaceLocation));
+ String tag = message.substring(spaceLocation+1);
- PluginDebug.debug("Inactive flag set. Refusing to initialize instance " + identifier);
- requests.remove(identifier);
- return;
-
- }
+ System.err.println("Handle = " + handle + "\n" +
+ "Width = " + width + "\n" +
+ "Height = " + height + "\n" +
+ "DocumentBase = " + documentBase + "\n" +
+ "Tag = " + tag);
status.put(identifier, PAV_INIT_STATUS.PRE_INIT);
+ PluginAppletViewer.parse
+ (identifier, handle, width, height,
+ new StringReader(tag),
+ new URL(documentBase));
- PluginParseRequest request = requests.get(identifier);
- if (request == null) {
- request = new PluginParseRequest();
- requests.put(identifier, request);
- }
- int index = message.indexOf(' ', "tag".length() + 1);
- request.documentbase =
- UrlUtil.decode(message.substring("tag".length() + 1, index));
- request.tag = message.substring(index + 1);
- PluginDebug.debug ("REQUEST TAG: " + request.tag + " " +
- Thread.currentThread());
- PluginDebug.debug ("REQUEST TAG, PARSING " +
- Thread.currentThread());
- PluginAppletViewer.parse
- (identifier, request.handle,
- new StringReader(request.tag),
- new URL(request.documentbase));
- requests.remove(identifier);
+ int maxWait = APPLET_TIMEOUT; // wait for applet to fully load
+ int wait = 0;
+ while (!status.get(identifier).equals(PAV_INIT_STATUS.INIT_COMPLETE) &&
+ (wait < maxWait)) {
- // 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");
- }
+ try {
+ Thread.sleep(50);
+ wait += 50;
+ } catch (InterruptedException ie) {
+ // just wait
+ }
}
+
+ if (!status.get(identifier).equals(PAV_INIT_STATUS.INIT_COMPLETE))
+ throw new Exception("Applet initialization timeout");
+
+ PluginAppletViewer oldFrame = applets.get(identifier);
+ reFrame(oldFrame, oldFrame.identifier, oldFrame.statusMsgStream,
+ handle, oldFrame.panel);
} else {
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
+ // Wait till initialization finishes
while (!applets.containsKey(identifier) &&
(
!status.containsKey(identifier) ||
@@ -617,37 +577,7 @@ import com.sun.jndi.toolkit.url.UrlUtil;
if (status.get(identifier).equals(PAV_INIT_STATUS.INACTIVE))
return;
- if (message.startsWith("handle")) {
-
- PluginDebug.debug("handle command waiting for applet to complete loading.");
- int maxWait = APPLET_TIMEOUT; // wait for applet to fully load
- int wait = 0;
- while (!status.get(identifier).equals(PAV_INIT_STATUS.INIT_COMPLETE) &&
- (wait < maxWait)) {
-
- try {
- Thread.sleep(50);
- wait += 50;
- } catch (InterruptedException ie) {
- // just wait
- }
- }
-
- if (!status.get(identifier).equals(PAV_INIT_STATUS.INIT_COMPLETE))
- throw new Exception("Applet initialization timeout");
-
- PluginDebug.debug("Applet loading complete. Proceeding to reframe.");
- long handle = Long.parseLong
- (message.substring("handle".length() + 1));
-
- PluginAppletViewer oldFrame = applets.get(identifier);
- reFrame(oldFrame, oldFrame.identifier, oldFrame.statusMsgStream,
- oldFrame.proposedHeightFactor, oldFrame.proposedWidthFactor,
- handle, oldFrame.panel);
-
- } else {
- applets.get(identifier).handleMessage(reference, message);
- }
+ applets.get(identifier).handleMessage(reference, message);
More information about the distro-pkg-dev
mailing list