[icedtea-web] RFC: Handle null NPP instances in plugin calls

[Deepak Bhole]

This patch fixes problems that may arise if a call is requested by an
already destroyed applet (race condition).

Additionally, it also does some very minor cleanup by removing an unused
thread count variable.

ChangeLog:

2011-03-02  Deepak Bhole <dbhole at redhat.com>

    * plugin/icedteanp/IcedTeaPluginRequestProcessor.cc
    (PluginRequestProcessor): Remove initialization of tc_mutex
    (~PluginRequestProcessor): Remove destruction of tc_mutex
    (eval): Proceed with _eval only if instance is valid.
    (call): Proceed with _call only if instance is valid. Moved initialization
    of result_variant_jniid and result_variant to the top.
    (sendString): Proceed with _getString only if instance is valid. Remove
    thread count incrementer.
    (setMember): Proceed with _setMember only if instance is valid. Remove
    thread count incrementer.
    (sendMember): Proceed with _getMember only if instance is valid. Remove
    thread count incrementer.
    * plugin/icedteanp/IcedTeaPluginRequestProcessor.h: Removed tc_mutex and
    thread_count variables.

1.0 is affected too, and I think the patch should be applied there too.

Cheers,
Deepak
-------------- next part --------------
diff -r 5dbf0f9de599 plugin/icedteanp/IcedTeaPluginRequestProcessor.cc
--- a/plugin/icedteanp/IcedTeaPluginRequestProcessor.cc	Wed Mar 02 11:50:30 2011 -0500
+++ b/plugin/icedteanp/IcedTeaPluginRequestProcessor.cc	Wed Mar 02 16:12:14 2011 -0500
@@ -66,7 +66,6 @@
 
     pthread_mutex_init(&message_queue_mutex, NULL);
     pthread_mutex_init(&syn_write_mutex, NULL);
-    pthread_mutex_init(&tc_mutex, NULL);
 
     pthread_cond_init(&cond_message_available, NULL);
 }
@@ -86,8 +85,6 @@
 
     pthread_mutex_destroy(&message_queue_mutex);
     pthread_mutex_destroy(&syn_write_mutex);
-    pthread_mutex_destroy(&tc_mutex);
-
     pthread_cond_destroy(&cond_message_available);
 }
 
@@ -236,6 +233,10 @@
     thread_data.parameters.push_back(NPVARIANT_TO_OBJECT(*window_ptr));
     thread_data.parameters.push_back(&script);
 
+    // If instance no longer exists, bail
+    if (!instance)
+        return;
+
 #ifdef CHROMIUM_WORKAROUND
     // Workaround for chromium
     _eval(&thread_data);
@@ -282,6 +283,8 @@
     std::string response = std::string();
     JavaRequestProcessor java_request = JavaRequestProcessor();
     JavaResultData* java_result;
+    std::string result_variant_jniid = std::string();
+    NPVariant* result_variant;
 
     reference = atoi(message_parts->at(3)->c_str());
 
@@ -331,6 +334,10 @@
     thread_data.parameters.push_back(&arg_count);
     thread_data.parameters.push_back(args_array);
 
+    // If instance no longer exists, bail
+    if (!instance)
+        goto cleanup;
+
 #ifdef CHROMIUM_WORKAROUND
     // Workaround for chromium
     _call(&thread_data);
@@ -346,8 +353,7 @@
     }
 #endif
 
-    NPVariant* result_variant = (NPVariant*) IcedTeaPluginUtilities::stringToJSID(thread_data.result);
-    std::string result_variant_jniid = std::string();
+    result_variant = (NPVariant*) IcedTeaPluginUtilities::stringToJSID(thread_data.result);
 
     if (result_variant)
     {
@@ -395,6 +401,10 @@
     thread_data.parameters.push_back(instance);
     thread_data.parameters.push_back(variant);
 
+    // If instance no longer exists, bail
+    if (!instance)
+        return;
+
 #ifdef CHROMIUM_WORKAROUND
     // Workaround for chromium
     _getString(&thread_data);
@@ -415,12 +425,6 @@
     response += thread_data.result;
 
     plugin_to_java_bus->post(response.c_str());
-
-    cleanup:
-
-    pthread_mutex_lock(&tc_mutex);
-    thread_count--;
-    pthread_mutex_unlock(&tc_mutex);
 }
 
 /**
@@ -490,6 +494,10 @@
     thread_data.parameters.push_back(&property_identifier);
     thread_data.parameters.push_back(&value);
 
+    // If instance no longer exists, bail
+    if (!instance)
+        return;
+
 #ifdef CHROMIUM_WORKAROUND
     // Workaround for chromium
     _setMember(&thread_data);
@@ -508,13 +516,6 @@
     IcedTeaPluginUtilities::constructMessagePrefix(0, reference, &response);
     response.append(" JavaScriptSetMember ");
     plugin_to_java_bus->post(response.c_str());
-
-    cleanup:
-
-    // property_name, type and value are deleted by _setMember
-    pthread_mutex_lock(&tc_mutex);
-    thread_count--;
-    pthread_mutex_unlock(&tc_mutex);
 }
 
 /**
@@ -588,6 +589,10 @@
     thread_data.parameters.push_back(NPVARIANT_TO_OBJECT(*parent_ptr));
     thread_data.parameters.push_back(&member_identifier);
 
+    // If instance no longer exists, bail
+    if (!instance)
+        return;
+
 #ifdef CHROMIUM_WORKAROUND
     // Workaround for chromium
     _getMember(&thread_data);
@@ -660,14 +665,6 @@
     }
     response.append(java_result->return_string->c_str());
     plugin_to_java_bus->post(response.c_str());
-
-
-    // Now be a good citizen and help keep the heap free of garbage
-    cleanup:
-
-    pthread_mutex_lock(&tc_mutex);
-    thread_count--;
-    pthread_mutex_unlock(&tc_mutex);
 }
 
 /**
diff -r 5dbf0f9de599 plugin/icedteanp/IcedTeaPluginRequestProcessor.h
--- a/plugin/icedteanp/IcedTeaPluginRequestProcessor.h	Wed Mar 02 11:50:30 2011 -0500
+++ b/plugin/icedteanp/IcedTeaPluginRequestProcessor.h	Wed Mar 02 16:12:14 2011 -0500
@@ -84,9 +84,6 @@
 void _eval(void* data);
 void _getString(void* data);
 
-static pthread_mutex_t tc_mutex = PTHREAD_MUTEX_INITIALIZER;
-static int thread_count = 0;
-
 void* queue_processor(void* data);
 
 /* Mutex to ensure that the request queue is accessed synchronously */