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

[Dr Andrew John Hughes]

On 16:45 Wed 02 Mar     , Deepak Bhole wrote:
> 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.
> 

I think the thread count removal should be in a separate patch, as it
seems unrelated to the other changes.

> 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

> 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 */


-- 
Andrew :)

Free Java Software Engineer
Red Hat, Inc. (http://www.redhat.com)

Support Free Java!
Contribute to GNU Classpath and IcedTea
http://www.gnu.org/software/classpath
http://icedtea.classpath.org
PGP Key: F5862A37 (https://keys.indymedia.org/)
Fingerprint = EA30 D855 D50F 90CD F54D  0698 0713 C3ED F586 2A37