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

[Deepak Bhole]

* Omair Majid <omajid at redhat.com> [2011-03-03 11:20]:
> On 03/03/2011 11:02 AM, Deepak Bhole wrote:
> 
> instance is not a volatile, nor is it a reference variable and it's
> never written (after being initialized). I cant figure out how it
> will change. Perhaps a you can add a comment explaining the change?
> 

Hmm not sure why I was thinking otherwise, but you are correct.
Attached are two separate patches that break down instance check and
cleanup.

Cheers,
Deepak

> >Thanks for reviewing! I will repost it later today (same content, just
> >broken down as Andrew suggested).
> >
> 
> That would be great.
> 
> Cheers,
> Omair
-------------- 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	Thu Mar 03 16:03:18 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,7 +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);
 }
@@ -415,12 +413,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);
 }
 
 /**
@@ -508,13 +500,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);
 }
 
 /**
@@ -660,14 +645,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	Thu Mar 03 16:03:18 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 */
-------------- 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	Thu Mar 03 16:26:04 2011 -0500
@@ -223,6 +221,10 @@
     window_ptr = (NPVariant*) IcedTeaPluginUtilities::stringToJSID(message_parts->at(5));
     instance = IcedTeaPluginUtilities::getInstanceFromMemberPtr(window_ptr);
 
+    // If instance is invalid, do not proceed further
+    if (!instance)
+    	return;
+
     java_result = request_processor.getString(*(message_parts->at(6)));
     CHECK_JAVA_RESULT(java_result);
     script.append(*(java_result->return_string));
@@ -282,6 +284,10 @@
     std::string response = std::string();
     JavaRequestProcessor java_request = JavaRequestProcessor();
     JavaResultData* java_result;
+    NPVariant* result_variant;
+    std::string result_variant_jniid = std::string();
+    NPVariant* args_array;
+    AsyncCallThreadData thread_data = AsyncCallThreadData();
 
     reference = atoi(message_parts->at(3)->c_str());
 
@@ -292,6 +298,10 @@
     // instance
     instance = IcedTeaPluginUtilities::getInstanceFromMemberPtr(window_ptr);
 
+    // If instance is invalid, do not proceed further
+    if (!instance)
+    	goto cleanup;
+
     // function name
     java_result = java_request.getString(*(message_parts->at(6)));
     CHECK_JAVA_RESULT(java_result);
@@ -316,11 +326,10 @@
     }
 
     arg_count = args.size();
-    NPVariant *args_array = (NPVariant*) malloc(sizeof(NPVariant)*args.size());
+    args_array = (NPVariant*) malloc(sizeof(NPVariant)*args.size());
     for (int i=0; i < args.size(); i++)
         args_array[i] = args[i];
 
-    AsyncCallThreadData thread_data = AsyncCallThreadData();
     thread_data.result_ready = false;
     thread_data.parameters = std::vector<void*>();
     thread_data.result = std::string();
@@ -346,8 +355,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)
     {
@@ -392,6 +400,11 @@
     thread_data.result = std::string();
 
     NPP instance = IcedTeaPluginUtilities::getInstanceFromMemberPtr(variant);
+
+    // If instance is invalid, do not proceed further
+    if (!instance)
+    	return;
+
     thread_data.parameters.push_back(instance);
     thread_data.parameters.push_back(variant);
 
@@ -463,6 +470,10 @@
 
     instance = IcedTeaPluginUtilities::getInstanceFromMemberPtr(member);
 
+    // If instance is invalid, do not proceed further
+    if (!instance)
+    	return;
+
     if (*(message_parts->at(4)) == "SetSlot")
     {
         property_identifier = browser_functions.getintidentifier(atoi(message_parts->at(6)->c_str()));
@@ -584,6 +588,11 @@
     thread_data.result = std::string();
 
     NPP instance = IcedTeaPluginUtilities::getInstanceFromMemberPtr(parent_ptr);
+
+    // If instance is invalid, do not proceed further
+    if (!instance)
+    	return;
+
     thread_data.parameters.push_back(instance);
     thread_data.parameters.push_back(NPVARIANT_TO_OBJECT(*parent_ptr));
     thread_data.parameters.push_back(&member_identifier);