Mercurial > hg > release > icedtea-web-1.5

changeset 987:b16b30a1380d

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
Fixed PR1743 - Intermittant deadlock in PluginRequestProcessor
author Jiri Vanek <jvanek@redhat.com>
date Thu, 19 Jun 2014 13:05:26 +0200
parents 576b9e693117
children 006c94731726
files AUTHORS ChangeLog NEWS plugin/icedteanp/IcedTeaNPPlugin.cc plugin/icedteanp/IcedTeaNPPlugin.h plugin/icedteanp/IcedTeaPluginRequestProcessor.cc plugin/icedteanp/IcedTeaPluginRequestProcessor.h
diffstat 7 files changed, 82 insertions(+), 57 deletions(-) [+]
line wrap: on
line diff
--- a/AUTHORS	Wed Jun 18 20:34:52 2014 +0200
+++ b/AUTHORS	Thu Jun 19 13:05:26 2014 +0200
@@ -8,12 +8,12 @@
 Danesh Dadachanji <ddadacha@redhat.com>
 Adam Domurad <adomurad@redhat.com>
 Thomas Fitzsimmons <fitzsim@redhat.com>
+Michał Górny < mgorny@gentoo.org >
 Mark Greenwood <mark@dcs.shef.ac.uk>
 Peter Hatina <phatina@redhat.com>
 Andrew John Hughes <ahughes@redhat.com>
 Matthias Klose <doko@ubuntu.com>
 Alexandr Kolouch <skolnag@gmail.com>
-Michał Górny < mgorny@gentoo.org >
 Jan Kmetko <jan.kmetko.ml@gmail.com>
 Francis Kung <fkung@redhat.com>
 Denis Lila <dlila@redhat.com>
@@ -21,6 +21,7 @@
 Omair Majid <omajid@redhat.com>
 Jon A. Maxwell <jmaxwell@users.sourceforge.net>
 Thomas Meyer <thomas@m3y3r.de>
+Kurt Miller <kurt@intricatesoftware.com>
 Saad Mohammad <smohammad@redhat.com>
 Martin Olsson  <martin@minimum.se>
 Andrew Su <asu@redhat.com>
--- a/ChangeLog	Wed Jun 18 20:34:52 2014 +0200
+++ b/ChangeLog	Thu Jun 19 13:05:26 2014 +0200
@@ -1,3 +1,23 @@
+2014-06-19  Kurt Miller <kurt@intricatesoftware.com>
+
+	Fixed PR1743 - Intermittant deadlock in PluginRequestProcessor
+	* NEWS: added PR1743
+	* plugin/icedteanp/IcedTeaNPPlugin.cc:  declaration of cond_message_available
+	moved to PluginRequestProcessor class
+	* plugin/icedteanp/IcedTeaNPPlugin.h: removed external cond_message_available search
+	* plugin/icedteanp/IcedTeaPluginRequestProcessor.h: message_queue_mutex,
+	syn_write_mutex and message_queue moved to PluginRequestProcessor clas. 
+	Constructor, destructor and newMessageOnBus declarationmoved to end of class.
+	declared queueProcessorThread method.
+	* plugin/icedteanp/IcedTeaPluginRequestProcessor.cc: Removed declaration of
+	static message_queue_mutex, syn_write_mutex, message_queue. (PluginRequestProcessor)
+	constructor and destructor and (newMessageOnBus) are now using the fields from
+	PluginRequestProcessor class. new method of (queue_wait_cleanup)  to unlock mutex
+	added. (queue_processor) is now calling queueProcessorThread. Implemented
+	(queueProcessorThread), which uses setMember, call , eval and loadUrl rather
+	then processor->, versions. If no message_parts are available,  the cleanup  is done
+	only if message_queue is empty.
+
 2014-06-18  Jacob Wisor  <gitne@gmx.de>
 
 	* netx/net/sourceforge/jnlp/resources/Messages.properties (BOredirect)
--- a/NEWS	Wed Jun 18 20:34:52 2014 +0200
+++ b/NEWS	Thu Jun 19 13:05:26 2014 +0200
@@ -9,6 +9,8 @@
 CVE-XXXX-YYYY: http://www.cve.mitre.org/cgi-bin/cvename.cgi?name=XXXX-YYYY
 
 New in release 1.5.1 (YYYY-MM-DD):
+* Plugin
+  - PR1743 - Intermittant deadlock in PluginRequestProcessor
 
 New in release 1.5 (2014-04-02):
 * IcedTea-Web now using tagsoup as default (tagsoup dependence) sanitizer for input
--- a/plugin/icedteanp/IcedTeaNPPlugin.cc	Wed Jun 18 20:34:52 2014 +0200
+++ b/plugin/icedteanp/IcedTeaNPPlugin.cc	Thu Jun 19 13:05:26 2014 +0200
@@ -239,8 +239,6 @@
 int plugin_debug_suspend = (getenv("ICEDTEAPLUGIN_DEBUG") != NULL) &&
         (strcmp(getenv("ICEDTEAPLUGIN_DEBUG"), "suspend") == 0);
 
-pthread_cond_t cond_message_available = PTHREAD_COND_INITIALIZER;
-
 
 #ifdef LEGACY_GLIB
 // Returns key from first item stored in hashtable
--- a/plugin/icedteanp/IcedTeaNPPlugin.h	Wed Jun 18 20:34:52 2014 +0200
+++ b/plugin/icedteanp/IcedTeaNPPlugin.h	Thu Jun 19 13:05:26 2014 +0200
@@ -106,9 +106,6 @@
 NPError initialize_data_directory();
 NPError start_jvm_if_needed();
 
-// 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;
 
--- a/plugin/icedteanp/IcedTeaPluginRequestProcessor.cc	Wed Jun 18 20:34:52 2014 +0200
+++ b/plugin/icedteanp/IcedTeaPluginRequestProcessor.cc	Thu Jun 19 13:05:26 2014 +0200
@@ -47,11 +47,6 @@
  * information, script execution and variable get/set
  */
 
-// Initialize static members used by the queue processing framework
-pthread_mutex_t message_queue_mutex = PTHREAD_MUTEX_INITIALIZER;
-pthread_mutex_t syn_write_mutex = PTHREAD_MUTEX_INITIALIZER;
-std::vector< std::vector<std::string*>* >* message_queue = new std::vector< std::vector<std::string*>* >();
-
 /**
  * PluginRequestProcessor constructor.
  *
@@ -60,14 +55,13 @@
 
 PluginRequestProcessor::PluginRequestProcessor()
 {
-    this->pendingRequests = new std::map<pthread_t, uintmax_t>();
+    this->message_queue = new std::vector< std::vector<std::string*>* >();
 
     internal_req_ref_counter = 0;
 
-    pthread_mutex_init(&message_queue_mutex, NULL);
-    pthread_mutex_init(&syn_write_mutex, NULL);
-
-    pthread_cond_init(&cond_message_available, NULL);
+    pthread_mutex_init(&this->message_queue_mutex, NULL);
+    pthread_mutex_init(&this->syn_write_mutex, NULL);
+    pthread_cond_init(&this->cond_message_available, NULL);
 }
 
 /**
@@ -80,12 +74,11 @@
 {
     PLUGIN_DEBUG("PluginRequestProcessor::~PluginRequestProcessor\n");
 
-    if (pendingRequests)
-        delete pendingRequests;
+    if (message_queue)
+        delete message_queue;
 
     pthread_mutex_destroy(&message_queue_mutex);
     pthread_mutex_destroy(&syn_write_mutex);
-
     pthread_cond_destroy(&cond_message_available);
 }
 
@@ -142,10 +135,9 @@
             // Update queue synchronously
             pthread_mutex_lock(&message_queue_mutex);
             message_queue->push_back(message_parts);
+            pthread_cond_signal(&cond_message_available);
             pthread_mutex_unlock(&message_queue_mutex);
 
-            // Broadcast that a message is now available
-            pthread_cond_broadcast(&cond_message_available);
 
             return true;
         }
@@ -635,10 +627,13 @@
 static void
 queue_cleanup(void* data)
 {
+    PLUGIN_DEBUG("Queue processing stopped.\n");
+}
 
-    pthread_mutex_destroy((pthread_mutex_t*) data);
-
-    PLUGIN_DEBUG("Queue processing stopped.\n");
+static void
+queue_wait_cleanup(void* data)
+{
+    pthread_mutex_unlock((pthread_mutex_t*) data);
 }
 
 void*
@@ -646,15 +641,19 @@
 {
 
     PluginRequestProcessor* processor = (PluginRequestProcessor*) data;
+    processor->queueProcessorThread();
+    return NULL;
+}
+
+void
+PluginRequestProcessor::queueProcessorThread()
+{
     std::vector<std::string*>* message_parts = NULL;
     std::string command;
-    pthread_mutex_t wait_mutex = PTHREAD_MUTEX_INITIALIZER;
 
     PLUGIN_DEBUG("Queue processor initialized. Queue = %p\n", message_queue);
 
-    pthread_mutex_init(&wait_mutex, NULL);
-
-    pthread_cleanup_push(queue_cleanup, (void*) &wait_mutex);
+    pthread_cleanup_push(queue_cleanup, NULL);
 
     while (true)
     {
@@ -672,45 +671,45 @@
 
             if (command == "GetMember")
             {
-                processor->sendMember(message_parts);
+                sendMember(message_parts);
             } else if (command == "ToString")
             {
-                processor->sendString(message_parts);
+                sendString(message_parts);
             } else if (command == "SetMember")
             {
             	// write methods are synchronized
             	pthread_mutex_lock(&syn_write_mutex);
-                processor->setMember(message_parts);
+                setMember(message_parts);
                 pthread_mutex_unlock(&syn_write_mutex);
             } else if (command == "Call")
             {
                 // write methods are synchronized
                 pthread_mutex_lock(&syn_write_mutex);
-                processor->call(message_parts);
+                call(message_parts);
                 pthread_mutex_unlock(&syn_write_mutex);
             } else if (command == "Eval")
             {
                 // write methods are synchronized
                 pthread_mutex_lock(&syn_write_mutex);
-                processor->eval(message_parts);
+                eval(message_parts);
                 pthread_mutex_unlock(&syn_write_mutex);
             } else if (command == "GetSlot")
             {
                 // write methods are synchronized
                 pthread_mutex_lock(&syn_write_mutex);
-                processor->sendMember(message_parts);
+                sendMember(message_parts);
                 pthread_mutex_unlock(&syn_write_mutex);
             } else if (command == "SetSlot")
             {
                 // write methods are synchronized
                 pthread_mutex_lock(&syn_write_mutex);
-                processor->setMember(message_parts);
+                setMember(message_parts);
                 pthread_mutex_unlock(&syn_write_mutex);
             } else if (command == "LoadURL") // For instance X url <url> <target>
             {
                 // write methods are synchronized
                 pthread_mutex_lock(&syn_write_mutex);
-                processor->loadURL(message_parts);
+                loadURL(message_parts);
                 pthread_mutex_unlock(&syn_write_mutex);
             } else
             {
@@ -723,9 +722,14 @@
 
         } else
         {
-	    pthread_mutex_lock(&wait_mutex);
-	    pthread_cond_wait(&cond_message_available, &wait_mutex);
-	    pthread_mutex_unlock(&wait_mutex);
+	    pthread_mutex_lock(&message_queue_mutex);
+            if (message_queue->size() == 0)
+            {
+	        pthread_cleanup_push(queue_wait_cleanup, &message_queue_mutex);
+	        pthread_cond_wait(&cond_message_available, &message_queue_mutex);
+	        pthread_cleanup_pop(0);
+            }
+	    pthread_mutex_unlock(&message_queue_mutex);
         }
 
         message_parts = NULL;
--- a/plugin/icedteanp/IcedTeaPluginRequestProcessor.h	Wed Jun 18 20:34:52 2014 +0200
+++ b/plugin/icedteanp/IcedTeaPluginRequestProcessor.h	Thu Jun 19 13:05:26 2014 +0200
@@ -69,15 +69,6 @@
 
 void* queue_processor(void* data);
 
-/* Mutex to ensure that the request queue is accessed synchronously */
-extern pthread_mutex_t message_queue_mutex;
-
-/* Mutex to ensure synchronized writes */
-extern pthread_mutex_t syn_write_mutex;
-
-/* Queue for holding messages that get processed in a separate thread */
-extern std::vector< std::vector<std::string*>* >* message_queue;
-
 /**
  * Processes requests made TO the plugin (by java or anyone else)
  */
@@ -85,8 +76,17 @@
 {
     private:
 
-    	/* Requests that are still pending */
-    	std::map<pthread_t, uintmax_t>* pendingRequests;
+        /* Mutex to ensure that the request queue is accessed synchronously */
+        pthread_mutex_t message_queue_mutex;
+
+        /* Condition on which the queue processor waits */
+        pthread_cond_t cond_message_available;
+
+        /* Queue for holding messages that get processed in a separate thread */
+        std::vector< std::vector<std::string*>* >* message_queue;
+
+        /* Mutex to ensure synchronized writes */
+        pthread_mutex_t syn_write_mutex;
 
     	/* Dispatch request processing to a new thread for asynch. processing */
     	void dispatch(void* func_ptr (void*), std::vector<std::string>* message, std::string* src);
@@ -97,13 +97,6 @@
         /* Stores the variant on java side */
     	void storeVariantInJava(NPVariant variant, std::string* result);
 
-    public:
-    	PluginRequestProcessor(); /* Constructor */
-    	~PluginRequestProcessor(); /* Destructor */
-
-    	/* Process new requests (if applicable) */
-        virtual bool newMessageOnBus(const char* message);
-
         /* Send member ID to Java */
         void sendMember(std::vector<std::string*>* message_parts);
 
@@ -124,6 +117,16 @@
 
         /* Loads a URL into the specified target */
         void loadURL(std::vector<std::string*>* message_parts);
+
+    public:
+    	PluginRequestProcessor(); /* Constructor */
+    	~PluginRequestProcessor(); /* Destructor */
+
+    	/* Process new requests (if applicable) */
+        virtual bool newMessageOnBus(const char* message);
+
+        /* Thread run method for processing queued messages */
+        void queueProcessorThread();
 };
 
 #endif // __ICEDTEAPLUGINREQUESTPROCESSOR_H__