Fix PR721: IcedTeaPlugin.so cannot run g_main_context_iteration on a different thread unless a different GMainContext *context is used

10 files changed, 258 insertions, 106 deletions

diff --git a/ChangeLog b/ChangeLog
index 60c9c31..198668c 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,51 @@
+2011-06-08  Deepak Bhole <[email protected]>
+
+	PR721: IcedTeaPlugin.so cannot run g_main_context_iteration on a different
+	thread unless a different GMainContext *context is used
+	* plugin/icedteanp/IcedTeaJavaRequestProcessor.cc
+	(postAndWaitForResponse): Added logic for tracking when the processor is
+	running from a plugin main thread, and logic to process main thread
+	specific messages queued thereafter until function exit.
+	* plugin/icedteanp/IcedTeaNPPlugin.cc:
+	(itnp_plugin_thread_id): New variable. Tracks plugin main thread ID.
+	(pluginAsyncCallMutex): New variable. Mutex to lock async call queue.
+	(NP_Initialize): Initialize the itnp_plugin_thread_id variable and make
+	ithe make pluginAsyncCallMutex recursive.
+	(NP_Shutdown): Destroy pluginAsyncCallMutex.
+	* plugin/icedteanp/IcedTeaNPPlugin.h:
+	(CHROMIUM_WORKAROUND): Remove macro.
+	(itnp_plugin_thread_id): New variable. Tracks plugin main thread ID.
+	(pluginAsyncCallMutex): New variable. Mutex to lock async call queue.
+	* plugin/icedteanp/IcedTeaPluginRequestProcessor.cc
+	(eval): Remove chromium workaround.
+	(call): Same.
+	(sendString): Same.
+	(setMember): Same.
+	(sendMember): Same.
+	(loadURL): Same.
+	* plugin/icedteanp/IcedTeaPluginRequestProcessor.h: Moved
+	async_call_thread_data to IcedTeaPluginUtils.h.
+	* plugin/icedteanp/IcedTeaPluginUtils.cc
+	(pendingPluginThreadRequests): New variable. Queue to track events waiting
+	for async execution on plug-in thread.
+	(callAndWaitForResult): New function. Calls a method on plug-in thread and
+	waits for the execution to complete.
+	(postPluginThreadAsyncCall): New function. Posts a method call to the
+	async execution queue and calls NPN_PluginThreadAsynCall.
+	(processAsyncCallQueue): New function. Called from the plug-in thread,
+	this function empties the event queue of functions waiting for plug-in
+	thread execution.
+	* plugin/icedteanp/IcedTeaPluginUtils.h
+	(plugin_thread_call): New struct to hold async call data.
+	(async_call_thread_data): Struct moved from IcedTeaPluginRequestProcessor.
+	(processAsyncCallQueue): New function.
+	(postPluginThreadAsyncCall): Same.
+	(callAndWaitForResult): Same.
+	* plugin/icedteanp/IcedTeaScriptablePluginObject.cc
+	(get_scriptable_java_object): Use
+	IcedTeaPluginUtilities::callAndWaitForResult to post async callback for
+	_createAndRetainJavaObject.
+
 2011-05-31  Omair Majid  <[email protected]>
 
 	* netx/net/sourceforge/jnlp/JNLPSplashScreen.java: Subclass JDialog, not
diff --git a/NEWS b/NEWS
index 8166c34..71dbbfa 100644
--- a/NEWS
+++ b/NEWS
@@ -33,6 +33,7 @@ New in release 1.1 (2011-XX-XX):
   - PR475, RH604061: Allow applets from the same page to use the same classloader
   - PR612: NetDania application ends on java.security.AccessControlException: access denied (java.util.PropertyPermission browser read)
   - PR664: Sound doesn't play on runescape.com.
+  - PR721: IcedTeaPlugin.so cannot run g_main_context_iteration on a different thread unless a different GMainContext *context is used
   - PR735: Firefox 4 sometimes freezes if the applet calls showDocument()
 
 New in release 1.0 (2010-XX-XX):
diff --git a/plugin/icedteanp/IcedTeaJavaRequestProcessor.cc b/plugin/icedteanp/IcedTeaJavaRequestProcessor.cc
index 8b5b8a3..5520206 100644
--- a/plugin/icedteanp/IcedTeaJavaRequestProcessor.cc
+++ b/plugin/icedteanp/IcedTeaJavaRequestProcessor.cc
@@ -247,22 +247,43 @@ JavaRequestProcessor::postAndWaitForResponse(std::string message)
     plugin_to_java_bus->post(message.c_str());
 
     // Wait for result to be filled in.
-	struct timespec curr_t;
+    struct timespec curr_t;
+
+    bool isPluginThread = false;
+
+    if (pthread_self() == itnp_plugin_thread_id)
+    {
+        isPluginThread = true;
+        PLUGIN_DEBUG("JRP is in plug-in thread...\n");
+    }
 
     do
     {
-    	clock_gettime(CLOCK_REALTIME, &curr_t);
+        clock_gettime(CLOCK_REALTIME, &curr_t);
 
-		if (!result_ready && (curr_t.tv_sec < t.tv_sec))
-		{
-			if (g_main_context_pending(NULL))
-				g_main_context_iteration(NULL, false);
-			else
-				usleep(200);
-		}
-		else
-			break;
+        if (!result_ready && (curr_t.tv_sec < t.tv_sec))
+        {
+            if (isPluginThread)
+            {
+                processAsyncCallQueue(NULL);
 
+                // Let the browser run its pending events too
+                if (g_main_context_pending(NULL))
+                {
+                    g_main_context_iteration(NULL, false);
+                } else
+                {
+                    usleep(1000); // 1ms
+                }
+            } else
+            {
+                usleep(1000); // 1ms
+            }
+        }
+        else
+        {
+            break;
+        }
     } while (1);
 
     if (curr_t.tv_sec >= t.tv_sec)
diff --git a/plugin/icedteanp/IcedTeaNPPlugin.cc b/plugin/icedteanp/IcedTeaNPPlugin.cc
index e9cd4c3..77a297e 100644
--- a/plugin/icedteanp/IcedTeaNPPlugin.cc
+++ b/plugin/icedteanp/IcedTeaNPPlugin.cc
@@ -160,6 +160,12 @@ gchar* out_pipe_name;
 // Applet viewer output watch source.
 gint out_watch_source;
 
+// Thread ID of plug-in thread
+pthread_t itnp_plugin_thread_id;
+
+// Mutex to lock async call queue
+pthread_mutex_t pluginAsyncCallMutex;
+
 // Applet viewer output channel.
 GIOChannel* out_to_appletviewer;
 
@@ -2204,8 +2210,6 @@ NP_Initialize (NPNetscapeFuncs* browserTable, NPPluginFuncs* pluginTable)
 
   PLUGIN_DEBUG ("NP_Initialize: using %s\n", appletviewer_executable);
 
-  PLUGIN_DEBUG ("NP_Initialize return\n");
-
   plugin_req_proc = new PluginRequestProcessor();
   java_req_proc = new JavaMessageSender();
 
@@ -2219,6 +2223,16 @@ NP_Initialize (NPNetscapeFuncs* browserTable, NPPluginFuncs* pluginTable)
   pthread_create (&plugin_request_processor_thread2, NULL, &queue_processor, (void*) plugin_req_proc);
   pthread_create (&plugin_request_processor_thread3, NULL, &queue_processor, (void*) plugin_req_proc);
 
+  itnp_plugin_thread_id = pthread_self();
+
+  pthread_mutexattr_t attribute;
+  pthread_mutexattr_init(&attribute);
+  pthread_mutexattr_settype(&attribute, PTHREAD_MUTEX_RECURSIVE);
+  pthread_mutex_init(&pluginAsyncCallMutex, &attribute);
+  pthread_mutexattr_destroy(&attribute);
+
+  PLUGIN_DEBUG ("NP_Initialize return\n");
+
   return NPERR_NO_ERROR;
 
  cleanup_appletviewer_executable:
@@ -2363,6 +2377,9 @@ NP_Shutdown (void)
   g_free (in_pipe_name);
   in_pipe_name = NULL;
 
+  // Destroy the call queue mutex
+  pthread_mutex_destroy(&pluginAsyncCallMutex);
+
   initialized = false;
 
   pthread_cancel(plugin_request_processor_thread1);
diff --git a/plugin/icedteanp/IcedTeaNPPlugin.h b/plugin/icedteanp/IcedTeaNPPlugin.h
index 18d1765..187aede 100644
--- a/plugin/icedteanp/IcedTeaNPPlugin.h
+++ b/plugin/icedteanp/IcedTeaNPPlugin.h
@@ -57,9 +57,6 @@ exception statement from your version. */
 #include "IcedTeaPluginUtils.h"
 #include "IcedTeaPluginRequestProcessor.h"
 
-// Work around across some chromium issues
-#define CHROMIUM_WORKAROUND
-
 // ITNPPluginData stores all the data associated with a single plugin
 // instance.  A separate plugin instance is created for each <APPLET>
 // tag.  For now, each plugin instance spawns its own applet viewer
@@ -97,6 +94,12 @@ static pthread_t plugin_request_processor_thread3;
 // Condition on which the queue processor waits
 extern pthread_cond_t cond_message_available;
 
+// ID of plug-in thread
+extern pthread_t itnp_plugin_thread_id;
+
+/* Mutex around plugin async call queue ops */
+extern pthread_mutex_t pluginAsyncCallMutex;
+
 // debug switch
 extern int plugin_debug;
 
diff --git a/plugin/icedteanp/IcedTeaPluginRequestProcessor.cc b/plugin/icedteanp/IcedTeaPluginRequestProcessor.cc
index 22b43b8..2e2b907 100644
--- a/plugin/icedteanp/IcedTeaPluginRequestProcessor.cc
+++ b/plugin/icedteanp/IcedTeaPluginRequestProcessor.cc
@@ -239,20 +239,7 @@ PluginRequestProcessor::eval(std::vector<std::string*>* message_parts)
     thread_data.parameters.push_back(NPVARIANT_TO_OBJECT(*window_ptr));
     thread_data.parameters.push_back(&script);
 
-#ifdef CHROMIUM_WORKAROUND
-    // Workaround for chromium
-    _eval(&thread_data);
-
-    if (!thread_data.call_successful)
-    {
-#endif
-        thread_data.result_ready = false;
-        browser_functions.pluginthreadasynccall(instance, &_eval, &thread_data);
-
-        while (!thread_data.result_ready) usleep(2000); // Wait till result is ready
-#ifdef CHROMIUM_WORKAROUND
-    }
-#endif
+    IcedTeaPluginUtilities::callAndWaitForResult(instance, &_eval, &thread_data);
 
     NPVariant* result_variant = (NPVariant*) IcedTeaPluginUtilities::stringToJSID(thread_data.result);
     std::string result_variant_jniid = std::string();
@@ -341,20 +328,7 @@ PluginRequestProcessor::call(std::vector<std::string*>* message_parts)
     thread_data.parameters.push_back(&arg_count);
     thread_data.parameters.push_back(args_array);
 
-#ifdef CHROMIUM_WORKAROUND
-    // Workaround for chromium
-    _call(&thread_data);
-
-    if (!thread_data.call_successful)
-    {
-#endif
-        thread_data.result_ready = false;
-        browser_functions.pluginthreadasynccall(instance, &_call, &thread_data);
-
-        while (!thread_data.result_ready) usleep(2000); // wait till ready
-#ifdef CHROMIUM_WORKAROUND
-    }
-#endif
+    IcedTeaPluginUtilities::callAndWaitForResult(instance, &_call, &thread_data);
 
     result_variant = (NPVariant*) IcedTeaPluginUtilities::stringToJSID(thread_data.result);
 
@@ -409,19 +383,7 @@ PluginRequestProcessor::sendString(std::vector<std::string*>* message_parts)
     thread_data.parameters.push_back(instance);
     thread_data.parameters.push_back(variant);
 
-#ifdef CHROMIUM_WORKAROUND
-    // Workaround for chromium
-    _getString(&thread_data);
-
-    if (!thread_data.call_successful)
-    {
-#endif
-        thread_data.result_ready = false;
-        browser_functions.pluginthreadasynccall(instance, &_getString, &thread_data);
-        while (!thread_data.result_ready) usleep(2000); // wait till ready
-#ifdef CHROMIUM_WORKAROUND
-    }
-#endif
+    IcedTeaPluginUtilities::callAndWaitForResult(instance, &_getString, &thread_data);
 
     // We need the context 0 for backwards compatibility with the Java side
     IcedTeaPluginUtilities::constructMessagePrefix(0, reference, &response);
@@ -502,20 +464,7 @@ PluginRequestProcessor::setMember(std::vector<std::string*>* message_parts)
     thread_data.parameters.push_back(&property_identifier);
     thread_data.parameters.push_back(&value);
 
-#ifdef CHROMIUM_WORKAROUND
-    // Workaround for chromium
-    _setMember(&thread_data);
-
-    if (!thread_data.call_successful)
-    {
-#endif
-        thread_data.result_ready = false;
-        browser_functions.pluginthreadasynccall(instance, &_setMember, &thread_data);
-
-        while (!thread_data.result_ready) usleep(2000); // wait till ready
-#ifdef CHROMIUM_WORKAROUND
-    }
-#endif
+    IcedTeaPluginUtilities::callAndWaitForResult(instance, &_setMember, &thread_data);
 
     IcedTeaPluginUtilities::constructMessagePrefix(0, reference, &response);
     response.append(" JavaScriptSetMember ");
@@ -598,21 +547,7 @@ PluginRequestProcessor::sendMember(std::vector<std::string*>* message_parts)
     thread_data.parameters.push_back(NPVARIANT_TO_OBJECT(*parent_ptr));
     thread_data.parameters.push_back(&member_identifier);
 
-#ifdef CHROMIUM_WORKAROUND
-    // Workaround for chromium
-    _getMember(&thread_data);
-
-    if (!thread_data.call_successful)
-    {
-#endif
-        thread_data.result_ready = false;
-        browser_functions.pluginthreadasynccall(instance, &_getMember, &thread_data);
-
-        while (!thread_data.result_ready) usleep(2000); // wait till ready
-
-#ifdef CHROMIUM_WORKAROUND
-    }
-#endif
+    IcedTeaPluginUtilities::callAndWaitForResult(instance, &_getMember, &thread_data);
 
     PLUGIN_DEBUG("Member PTR after internal request: %s\n", thread_data.result.c_str());
 
@@ -743,8 +678,7 @@ PluginRequestProcessor::loadURL(std::vector<std::string*>* message_parts)
     thread_data.parameters.push_back(message_parts->at(6)); // push target
 
     thread_data.result_ready = false;
-    browser_functions.pluginthreadasynccall(instance, &_loadURL, &thread_data);
-    while (!thread_data.result_ready) usleep(2000); // wait till ready
+    IcedTeaPluginUtilities::callAndWaitForResult(instance, &_loadURL, &thread_data);
 }
 
 static void
diff --git a/plugin/icedteanp/IcedTeaPluginRequestProcessor.h b/plugin/icedteanp/IcedTeaPluginRequestProcessor.h
index 8ee4e2d..e9c1f5d 100644
--- a/plugin/icedteanp/IcedTeaPluginRequestProcessor.h
+++ b/plugin/icedteanp/IcedTeaPluginRequestProcessor.h
@@ -57,18 +57,6 @@ exception statement from your version. */
 #include "IcedTeaPluginUtils.h"
 #include "IcedTeaJavaRequestProcessor.h"
 
-/**
- * Data structure passed to functions called in a new thread.
- */
-
-typedef struct async_call_thread_data
-{
-    std::vector<void*> parameters;
-	std::string result;
-	bool result_ready;
-	bool call_successful;
-} AsyncCallThreadData;
-
 /* Internal request reference counter */
 static long internal_req_ref_counter;
 
@@ -109,10 +97,10 @@ class PluginRequestProcessor : public BusSubscriber
     	/* Dispatch request processing to a new thread for asynch. processing */
     	void dispatch(void* func_ptr (void*), std::vector<std::string>* message, std::string* src);
 
-    	/* Send main window pointer to Java */
-    	void sendWindow(std::vector<std::string*>* message_parts);
+        /* Send main window pointer to Java */
+        void sendWindow(std::vector<std::string*>* message_parts);
 
-    	/* Stores the variant on java side */
+        /* Stores the variant on java side */
     	void storeVariantInJava(NPVariant variant, std::string* result);
 
     public:
diff --git a/plugin/icedteanp/IcedTeaPluginUtils.cc b/plugin/icedteanp/IcedTeaPluginUtils.cc
index 69daef2..854430d 100644
--- a/plugin/icedteanp/IcedTeaPluginUtils.cc
+++ b/plugin/icedteanp/IcedTeaPluginUtils.cc
@@ -54,6 +54,9 @@ pthread_mutex_t IcedTeaPluginUtilities::reference_mutex = PTHREAD_MUTEX_INITIALI
 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*>();
 
+/* Plugin async call queue */
+static std::vector< PluginThreadCall* >* pendingPluginThreadRequests = new std::vector< PluginThreadCall* >();
+
 /**
  * Given a context number, constructs a message prefix to send to Java
  *
@@ -910,6 +913,108 @@ IcedTeaPluginUtilities::decodeURL(const gchar* url, gchar** decoded_url)
     PLUGIN_DEBUG("SENDING URL: %s\n", *decoded_url);
 }
 
+
+/**
+ * Posts a function for execution on the plug-in thread and wait for result.
+ *
+ * @param instance The NPP instance
+ * @param func The function to post
+ * @param data Arguments to *func
+ */
+void
+IcedTeaPluginUtilities::callAndWaitForResult(NPP instance, void (*func) (void *), AsyncCallThreadData* data)
+{
+
+    struct timespec t;
+    struct timespec curr_t;
+    clock_gettime(CLOCK_REALTIME, &t);
+    t.tv_sec += REQUESTTIMEOUT; // timeout
+
+    // post request
+    postPluginThreadAsyncCall(instance, func, data);
+
+    do
+    {
+        clock_gettime(CLOCK_REALTIME, &curr_t);
+        if (data != NULL && !data->result_ready && (curr_t.tv_sec < t.tv_sec))
+        {
+            usleep(2000);
+        } else
+        {
+            break;
+        }
+    } while (1);
+}
+
+
+/**
+ * Posts a request that needs to be handled in a plugin thread.
+ *
+ * @param instance The plugin instance
+ * @param func The function to execute
+ * @param userData The userData for the function to consume/write to
+ * @return if the call was posted successfully
+ */
+
+bool
+IcedTeaPluginUtilities::postPluginThreadAsyncCall(NPP instance, void (*func) (void *), void* data)
+{
+    if (instance)
+    {
+        PluginThreadCall* call = new PluginThreadCall();
+        call->instance = instance;
+        call->func = func;
+        call->userData = data;
+
+        pthread_mutex_lock(&pluginAsyncCallMutex);
+        pendingPluginThreadRequests->push_back(call);
+        pthread_mutex_unlock(&pluginAsyncCallMutex);
+
+        browser_functions.pluginthreadasynccall(instance, &processAsyncCallQueue, NULL); // Always returns immediately
+
+        PLUGIN_DEBUG("Pushed back call evt %p\n", call);
+
+        return true;
+    }
+
+    // Else
+    PLUGIN_DEBUG("Instance is not active. Call rejected.\n");
+    return false;
+}
+
+/**
+ * Runs through the async call wait queue and executes all calls
+ *
+ * @param param Ignored -- required to conform to NPN_PluginThreadAsynCall API
+ */
+void
+processAsyncCallQueue(void* param /* ignored */)
+{
+    do {
+        PluginThreadCall* call = NULL;
+
+        pthread_mutex_lock(&pluginAsyncCallMutex);
+        if (pendingPluginThreadRequests->size() > 0)
+        {
+            call = pendingPluginThreadRequests->front();
+            pendingPluginThreadRequests->erase(pendingPluginThreadRequests->begin());
+        }
+        pthread_mutex_unlock(&pluginAsyncCallMutex);
+
+        if (call)
+        {
+            PLUGIN_DEBUG("Processing call evt %p\n", call);
+            call->func(call->userData);
+            PLUGIN_DEBUG("Call evt %p processed\n", call);
+
+            delete call;
+        } else
+        {
+            break;
+        }
+    } while(1);
+}
+
 /******************************************
  * Begin JavaMessageSender implementation *
  ******************************************
diff --git a/plugin/icedteanp/IcedTeaPluginUtils.h b/plugin/icedteanp/IcedTeaPluginUtils.h
index 8584fb8..6a168da 100644
--- a/plugin/icedteanp/IcedTeaPluginUtils.h
+++ b/plugin/icedteanp/IcedTeaPluginUtils.h
@@ -119,12 +119,43 @@ typedef struct java_result_data
 
 } JavaResultData;
 
+/**
+ * This struct holds data to do calls that need to be run in the plugin thread
+ */
+typedef struct plugin_thread_call
+{
+   // The plugin instance
+   NPP instance;
+
+   // The function to call
+   void (*func) (void *);
+
+   // The data to pass to the function
+   void *userData;
+
+} PluginThreadCall;
+
+/**
+ * Data structure passed to functions called in a new thread.
+ */
+
+typedef struct async_call_thread_data
+{
+    std::vector<void*> parameters;
+    std::string result;
+    bool result_ready;
+    bool call_successful;
+} AsyncCallThreadData;
+
 /*
  * Misc. utility functions
  *
  * This class is never instantiated and should contain static functions only
  */
 
+/* Function to process all pending async calls */
+void processAsyncCallQueue(void*);
+
 class IcedTeaPluginUtilities
 {
 
@@ -140,6 +171,9 @@ class IcedTeaPluginUtilities
         /* Map holding java-side-obj-key->NPObject relationship  */
         static std::map<std::string, NPObject*>* object_map;
 
+        /* Posts a call in the async call queue */
+        static bool postPluginThreadAsyncCall(NPP instance, void (*func) (void *), void* data);
+
     public:
 
     	/* Constructs message prefix with given context */
@@ -227,6 +261,9 @@ class IcedTeaPluginUtilities
     	static bool isObjectJSArray(NPP instance, NPObject* object);
 
     	static void decodeURL(const char* url, char** decoded_url);
+
+    	/* Posts call in async queue and waits till execution completes */
+    	static void callAndWaitForResult(NPP instance, void (*func) (void *), AsyncCallThreadData* data);
 };
 
 /*
diff --git a/plugin/icedteanp/IcedTeaScriptablePluginObject.cc b/plugin/icedteanp/IcedTeaScriptablePluginObject.cc
index e482941..518fec6 100644
--- a/plugin/icedteanp/IcedTeaScriptablePluginObject.cc
+++ b/plugin/icedteanp/IcedTeaScriptablePluginObject.cc
@@ -411,9 +411,7 @@ IcedTeaScriptableJavaPackageObject::get_scriptable_java_object(NPP instance,
         thread_data.parameters.push_back(np_class);
         thread_data.parameters.push_back(&scriptable_object);
 
-        browser_functions.pluginthreadasynccall(instance, &_createAndRetainJavaObject, &thread_data);
-
-        while (!thread_data.result_ready) usleep(2000); // wait till ready
+        IcedTeaPluginUtilities::callAndWaitForResult(instance, &_createAndRetainJavaObject, &thread_data);
     } else
     {
         // Else retain object and continue