[RFC] Single threaded JSRuntime in Firefox 10
Deepak Bhole
dbhole at redhat.com
Fri Feb 17 15:25:06 PST 2012
* Deepak Bhole <dbhole at redhat.com> [2012-02-16 17:59]:
> * Deepak Bhole <dbhole at redhat.com> [2012-02-16 14:53]:
> > * Thomas Meyer <thomas at m3y3r.de> [2012-02-03 20:20]:
> > > Hello,
> > >
> > > please have a look at this first patch that fixes the sendMember() and
> > > eval() methods for firefox 10:
> > >
> >
> > Hi Thomas,
> >
> > I don't think these will actually fix the issue. The issue is
> > intermittent, so your fixes are probably just reducing the frequency.
> >
> > The problem is the use of the browser_function.* functions from non-JS
> > threads.
> >
> > I am working on a patch to resolve it. However your patch is still a
> > good re-factor and I think it would make a good addition on top :)
> >
> > I'll reply back after I've posted the fix.
> >
>
> Sorry, scratch that. Looking at your fixes closer, I see you have moved
> the browser calls to the functions that are guaranteed to execute on
> main JS engine thread.
>
> I will take a look at this first thing tomorrow and fix up anything left
> by patch 1 and 2.
>
Hi Thomas,
I have reviewed your changes and found an issue (nothing you did wrong,
your fixes just exposed it) related to array processing (in
createJavaObjectFromVariant()).
I have attached a final patch that has your fixes + that fix + fixes to
finalize as they all go hand in hand.
sendWindow() does not need a fix because it always runs on the main
thread. I just changed finalize() to do the same as neither can create a
chain of recursive calls.
New patch attached.
Again, thank you very much for the patch! :)
If there are no comments by Tuesday, I think it should be okay to push.
Cheers,
Deepak
> Thanks!
> Deepak
>
> > Thanks,
> > Deepak
> >
> > > # HG changeset patch
> > > # User thomas
> > > # Date 1328317526 -3600
> > > # Node ID 6962b636cfdaeef1efa884bcd2207a1360baa978
> > > # Parent 528e354ff46919a758c8977978053fbb816ffebc
> > > Fix eval() and sendMember() for the new single thread JSRuntime in Firefox 10. See also:
> > > - https://bugzilla.mozilla.org/show_bug.cgi?id=704249
> > > - http://icedtea.classpath.org/bugzilla/show_bug.cgi?id=820
> > > - http://groups.google.com/group/mozilla.dev.tech.js-engine/msg/ae5f22f39e4fd150
> > >
> > > diff -r 528e354ff469 -r 6962b636cfda plugin/icedteanp/IcedTeaPluginRequestProcessor.cc
> > > --- a/plugin/icedteanp/IcedTeaPluginRequestProcessor.cc Thu Feb 02 16:15:27 2012 -0500
> > > +++ b/plugin/icedteanp/IcedTeaPluginRequestProcessor.cc Sat Feb 04 02:05:26 2012 +0100
> > > @@ -241,19 +241,9 @@
> > >
> > > IcedTeaPluginUtilities::callAndWaitForResult(instance, &_eval, &thread_data);
> > >
> > > - NPVariant* result_variant = (NPVariant*) IcedTeaPluginUtilities::stringToJSID(thread_data.result);
> > > - std::string result_variant_jniid = std::string();
> > > - if (result_variant)
> > > - {
> > > - createJavaObjectFromVariant(instance, *result_variant, &result_variant_jniid);
> > > - } else
> > > - {
> > > - result_variant_jniid = "0";
> > > - }
> > > -
> > > IcedTeaPluginUtilities::constructMessagePrefix(0, reference, &response);
> > > response += " JavaScriptEval ";
> > > - response += result_variant_jniid;
> > > + response += thread_data.result;
> > >
> > > plugin_to_java_bus->post(response.c_str());
> > > }
> > > @@ -522,7 +512,7 @@
> > > /** Request data from Java if necessary **/
> > > if (*(message_parts->at(4)) == "GetSlot")
> > > {
> > > - member_identifier = browser_functions.getintidentifier(atoi(member_id.c_str()));
> > > + //member_id = ;
> > > } else
> > > {
> > > // make a new request for getString, to get the name of the identifier
> > > @@ -535,7 +525,7 @@
> > > //goto cleanup;
> > > }
> > >
> > > - member_identifier = browser_functions.getstringidentifier(java_result->return_string->c_str());
> > > + member_id = *java_result->return_string;
> > > }
> > >
> > > AsyncCallThreadData thread_data = AsyncCallThreadData();
> > > @@ -551,16 +541,10 @@
> > >
> > > thread_data.parameters.push_back(instance);
> > > thread_data.parameters.push_back(NPVARIANT_TO_OBJECT(*parent_ptr));
> > > - thread_data.parameters.push_back(&member_identifier);
> > > + thread_data.parameters.push_back(&member_id);
> > >
> > > IcedTeaPluginUtilities::callAndWaitForResult(instance, &_getMember, &thread_data);
> > >
> > > - PLUGIN_DEBUG("Member PTR after internal request: %s\n", thread_data.result.c_str());
> > > -
> > > - member_ptr = (NPVariant*) IcedTeaPluginUtilities::stringToJSID(thread_data.result);
> > > -
> > > - createJavaObjectFromVariant(instance, *member_ptr, &result_id);
> > > -
> > > IcedTeaPluginUtilities::constructMessagePrefix(0, reference, &response);
> > > if (*(message_parts->at(4)) == "GetSlot")
> > > {
> > > @@ -568,7 +552,7 @@
> > > } else {
> > > response.append(" JavaScriptGetMember ");
> > > }
> > > - response.append(result_id.c_str());
> > > + response.append(thread_data.result);
> > > plugin_to_java_bus->post(response.c_str());
> > > }
> > >
> > > @@ -797,23 +781,27 @@
> > >
> > > instance = (NPP) parameters.at(0);
> > > parent_ptr = (NPObject*) parameters.at(1);
> > > - NPIdentifier* member_identifier = (NPIdentifier*) parameters.at(2);
> > > + std::string* member_id = (std::string*) parameters.at(2);
> > > + NPIdentifier member_identifier;
> > > +
> > > + member_identifier = browser_functions.getintidentifier(atoi(member_id->c_str()));
> > >
> > > // Get the NPVariant corresponding to this member
> > > - PLUGIN_DEBUG("Looking for %p %p %p (%s)\n", instance, parent_ptr, member_identifier, browser_functions.utf8fromidentifier(*member_identifier));
> > > + PLUGIN_DEBUG("Looking for %p %p %p (%s)\n", instance, parent_ptr, member_identifier, browser_functions.utf8fromidentifier(member_identifier));
> > >
> > > - if (!browser_functions.hasproperty(instance, parent_ptr, *member_identifier))
> > > + if (!browser_functions.hasproperty(instance, parent_ptr, member_identifier))
> > > {
> > > - printf("%s not found!\n", browser_functions.utf8fromidentifier(*member_identifier));
> > > + printf("%s not found!\n", browser_functions.utf8fromidentifier(member_identifier));
> > > }
> > > - ((AsyncCallThreadData*) data)->call_successful = browser_functions.getproperty(instance, parent_ptr, *member_identifier, member_ptr);
> > > + ((AsyncCallThreadData*) data)->call_successful = browser_functions.getproperty(instance, parent_ptr, member_identifier, member_ptr);
> > >
> > > IcedTeaPluginUtilities::printNPVariant(*member_ptr);
> > >
> > > if (((AsyncCallThreadData*) data)->call_successful)
> > > {
> > > - IcedTeaPluginUtilities::JSIDToString(member_ptr, &member_ptr_str);
> > > + createJavaObjectFromVariant(instance, *member_ptr, &member_ptr_str);
> > > ((AsyncCallThreadData*) data)->result.append(member_ptr_str);
> > > +
> > > }
> > > ((AsyncCallThreadData*) data)->result_ready = true;
> > >
> > > @@ -831,8 +819,8 @@
> > > std::string* script_str;
> > > NPIdentifier script_identifier;
> > > NPString script = NPString();
> > > - NPVariant* eval_result = new NPVariant();
> > > - std::string eval_result_ptr_str = std::string();
> > > + NPVariant* eval_variant = new NPVariant();
> > > + std::string eval_variant_str = std::string();
> > >
> > > PLUGIN_DEBUG("_eval called\n");
> > >
> > > @@ -854,13 +842,19 @@
> > > PLUGIN_DEBUG("Evaluating: %s\n", script.UTF8Characters);
> > > #endif
> > >
> > > - ((AsyncCallThreadData*) data)->call_successful = browser_functions.evaluate(instance, window_ptr, &script, eval_result);
> > > - IcedTeaPluginUtilities::printNPVariant(*eval_result);
> > > + ((AsyncCallThreadData*) data)->call_successful = browser_functions.evaluate(instance, window_ptr, &script, eval_variant);
> > > + IcedTeaPluginUtilities::printNPVariant(*eval_variant);
> > >
> > > if (((AsyncCallThreadData*) data)->call_successful)
> > > {
> > > - IcedTeaPluginUtilities::JSIDToString(eval_result, &eval_result_ptr_str);
> > > - ((AsyncCallThreadData*) data)->result.append(eval_result_ptr_str);
> > > + if (eval_variant)
> > > + {
> > > + createJavaObjectFromVariant(instance, *eval_variant, &eval_variant_str);
> > > + } else
> > > + {
> > > + eval_variant_str = "0";
> > > + }
> > > + ((AsyncCallThreadData*) data)->result.append(eval_variant_str);
> > > }
> > > ((AsyncCallThreadData*) data)->result_ready = true;
> > >
> > >
> > >
-------------- next part --------------
diff -r 360bd0a75304 plugin/icedteanp/IcedTeaJavaRequestProcessor.cc
--- a/plugin/icedteanp/IcedTeaJavaRequestProcessor.cc Mon Jan 09 18:45:31 2012 -0500
+++ b/plugin/icedteanp/IcedTeaJavaRequestProcessor.cc Fri Feb 17 18:24:39 2012 -0500
@@ -921,6 +921,7 @@
{
PLUGIN_DEBUG("NPObject is not a Java object\n");
NPIdentifier length_id = browser_functions.getstringidentifier("length");
+ bool isJSObjectArray = false;
// FIXME: We currently only handle <= 2 dim arrays. Do we really need more though?
@@ -936,7 +937,7 @@
std::string length_str = std::string();
IcedTeaPluginUtilities::itoa(NPVARIANT_TO_INT32(length), &length_str);
- if (NPVARIANT_TO_INT32(length) > 0)
+ if (NPVARIANT_TO_INT32(length) >= 0)
{
NPIdentifier id_0 = browser_functions.getintidentifier(0);
NPVariant first_element = NPVariant();
@@ -956,8 +957,14 @@
{
getArrayTypeForJava(instance, first_element, &java_array_type);
}
- } else
- java_array_type.append("jsobject");
+ }
+
+ // For JSObject arrays, we create a regular object (accessible via JSObject.getSlot())
+ if (NPVARIANT_TO_INT32(length) < 0 || !java_array_type.compare("jsobject"))
+ {
+ isJSObjectArray = true;
+ goto createRegularObject;
+ }
java_result = java_request.newArray(java_array_type, length_str);
@@ -995,7 +1002,10 @@
// Got here => no errors above. We're good to return!
return;
- } else // Else it is not an array
+ }
+
+ createRegularObject:
+ if (!IcedTeaPluginUtilities::isObjectJSArray(instance, obj) || isJSObjectArray) // Else it is not an array
{
NPVariant* variant_copy = new NPVariant();
diff -r 360bd0a75304 plugin/icedteanp/IcedTeaPluginRequestProcessor.cc
--- a/plugin/icedteanp/IcedTeaPluginRequestProcessor.cc Mon Jan 09 18:45:31 2012 -0500
+++ b/plugin/icedteanp/IcedTeaPluginRequestProcessor.cc Fri Feb 17 18:24:39 2012 -0500
@@ -123,6 +123,12 @@
// returns immediately, so we do it in the same thread.
this->sendWindow(message_parts);
return true;
+ } else if (!command->find("Finalize"))
+ {
+ // Object can be finalized from the main thread only. And this
+ // call returns immediately, so we do it in the same thread.
+ this->finalize(message_parts);
+ return true;
} else if (!command->find("GetMember") ||
!command->find("SetMember") ||
!command->find("ToString") ||
@@ -130,7 +136,6 @@
!command->find("GetSlot") ||
!command->find("SetSlot") ||
!command->find("Eval") ||
- !command->find("Finalize") ||
!command->find("LoadURL"))
{
@@ -241,19 +246,9 @@
IcedTeaPluginUtilities::callAndWaitForResult(instance, &_eval, &thread_data);
- NPVariant* result_variant = (NPVariant*) IcedTeaPluginUtilities::stringToJSID(thread_data.result);
- std::string result_variant_jniid = std::string();
- if (result_variant)
- {
- createJavaObjectFromVariant(instance, *result_variant, &result_variant_jniid);
- } else
- {
- result_variant_jniid = "0";
- }
-
IcedTeaPluginUtilities::constructMessagePrefix(0, reference, &response);
response += " JavaScriptEval ";
- response += result_variant_jniid;
+ response += thread_data.result;
plugin_to_java_bus->post(response.c_str());
}
@@ -336,19 +331,9 @@
IcedTeaPluginUtilities::callAndWaitForResult(instance, &_call, &thread_data);
- result_variant = (NPVariant*) IcedTeaPluginUtilities::stringToJSID(thread_data.result);
-
- if (result_variant)
- {
- createJavaObjectFromVariant(instance, *result_variant, &result_variant_jniid);
- } else
- {
- result_variant_jniid = "0";
- }
-
IcedTeaPluginUtilities::constructMessagePrefix(0, reference, &response);
response += " JavaScriptCall ";
- response += result_variant_jniid;
+ response += thread_data.result;
plugin_to_java_bus->post(response.c_str());
@@ -415,7 +400,8 @@
NPP instance;
NPVariant* member;
- NPIdentifier property_identifier;
+ std::string property_id = std::string();
+ bool int_identifier;
JavaRequestProcessor java_request = JavaRequestProcessor();
JavaResultData* java_result;
@@ -445,7 +431,8 @@
if (*(message_parts->at(4)) == "SetSlot")
{
- property_identifier = browser_functions.getintidentifier(atoi(message_parts->at(6)->c_str()));
+ property_id.append(*(message_parts->at(6)));
+ int_identifier = true;
} else
{
java_result = java_request.getString(propertyNameID);
@@ -457,7 +444,8 @@
//goto cleanup;
}
- property_identifier = browser_functions.getstringidentifier(java_result->return_string->c_str());
+ property_id.append(*(java_result->return_string));
+ int_identifier = false;
}
AsyncCallThreadData thread_data = AsyncCallThreadData();
@@ -467,8 +455,9 @@
thread_data.parameters.push_back(instance);
thread_data.parameters.push_back(NPVARIANT_TO_OBJECT(*member));
- thread_data.parameters.push_back(&property_identifier);
+ thread_data.parameters.push_back(&property_id);
thread_data.parameters.push_back(&value);
+ thread_data.parameters.push_back(&int_identifier);
IcedTeaPluginUtilities::callAndWaitForResult(instance, &_setMember, &thread_data);
@@ -508,6 +497,7 @@
int method_id;
int instance_id;
int reference;
+ bool int_identifier;
// debug printout of parent thread data
IcedTeaPluginUtilities::printStringPtrVector("PluginRequestProcessor::getMember:", message_parts);
@@ -522,7 +512,7 @@
/** Request data from Java if necessary **/
if (*(message_parts->at(4)) == "GetSlot")
{
- member_identifier = browser_functions.getintidentifier(atoi(member_id.c_str()));
+ int_identifier=true;
} else
{
// make a new request for getString, to get the name of the identifier
@@ -535,7 +525,8 @@
//goto cleanup;
}
- member_identifier = browser_functions.getstringidentifier(java_result->return_string->c_str());
+ member_id.assign(*(java_result->return_string));
+ int_identifier=false;
}
AsyncCallThreadData thread_data = AsyncCallThreadData();
@@ -551,16 +542,11 @@
thread_data.parameters.push_back(instance);
thread_data.parameters.push_back(NPVARIANT_TO_OBJECT(*parent_ptr));
- thread_data.parameters.push_back(&member_identifier);
+ thread_data.parameters.push_back(&member_id);
+ thread_data.parameters.push_back(&int_identifier);
IcedTeaPluginUtilities::callAndWaitForResult(instance, &_getMember, &thread_data);
- PLUGIN_DEBUG("Member PTR after internal request: %s\n", thread_data.result.c_str());
-
- member_ptr = (NPVariant*) IcedTeaPluginUtilities::stringToJSID(thread_data.result);
-
- createJavaObjectFromVariant(instance, *member_ptr, &result_id);
-
IcedTeaPluginUtilities::constructMessagePrefix(0, reference, &response);
if (*(message_parts->at(4)) == "GetSlot")
{
@@ -568,7 +554,7 @@
} else {
response.append(" JavaScriptGetMember ");
}
- response.append(result_id.c_str());
+ response.append(thread_data.result);
plugin_to_java_bus->post(response.c_str());
}
@@ -768,19 +754,26 @@
NPP instance;
NPVariant value_variant = NPVariant();
NPObject* member;
- NPIdentifier* property;
+ NPIdentifier property_identifier;
+
std::vector<void*> parameters = ((AsyncCallThreadData*) data)->parameters;
instance = (NPP) parameters.at(0);
member = (NPObject*) parameters.at(1);
- property = (NPIdentifier*) parameters.at(2);
+ std::string* property_id = (std::string*) parameters.at(2);
value = (std::string*) parameters.at(3);
+ bool* int_identifier = (bool*) parameters.at(4);
- PLUGIN_DEBUG("Setting %s on instance %p, object %p to value %s\n", browser_functions.utf8fromidentifier(*property), instance, member, value->c_str());
+ if(*int_identifier==true)
+ property_identifier = browser_functions.getintidentifier(atoi(property_id->c_str()));
+ else
+ property_identifier = browser_functions.getstringidentifier(property_id->c_str());
+
+ PLUGIN_DEBUG("Setting %s on instance %p, object %p to value %s\n", browser_functions.utf8fromidentifier(property_identifier), instance, member, value->c_str());
IcedTeaPluginUtilities::javaResultToNPVariant(instance, value, &value_variant);
- ((AsyncCallThreadData*) data)->call_successful = browser_functions.setproperty(instance, member, *property, &value_variant);
+ ((AsyncCallThreadData*) data)->call_successful = browser_functions.setproperty(instance, member, property_identifier, &value_variant);
((AsyncCallThreadData*) data)->result_ready = true;
}
@@ -797,23 +790,32 @@
instance = (NPP) parameters.at(0);
parent_ptr = (NPObject*) parameters.at(1);
- NPIdentifier* member_identifier = (NPIdentifier*) parameters.at(2);
+ std::string* member_id = (std::string*) parameters.at(2);
+ NPIdentifier member_identifier;
+
+ bool* int_identifier = (bool*) parameters.at(3);
+
+ if(*int_identifier==true)
+ member_identifier = browser_functions.getintidentifier(atoi(member_id->c_str()));
+ else
+ member_identifier = browser_functions.getstringidentifier(member_id->c_str());
// Get the NPVariant corresponding to this member
- PLUGIN_DEBUG("Looking for %p %p %p (%s)\n", instance, parent_ptr, member_identifier, browser_functions.utf8fromidentifier(*member_identifier));
+ PLUGIN_DEBUG("Looking for %p %p %p (%s)\n", instance, parent_ptr, member_identifier, browser_functions.utf8fromidentifier(member_identifier));
- if (!browser_functions.hasproperty(instance, parent_ptr, *member_identifier))
+ if (!browser_functions.hasproperty(instance, parent_ptr, member_identifier))
{
- printf("%s not found!\n", browser_functions.utf8fromidentifier(*member_identifier));
+ printf("%s not found!\n", browser_functions.utf8fromidentifier(member_identifier));
}
- ((AsyncCallThreadData*) data)->call_successful = browser_functions.getproperty(instance, parent_ptr, *member_identifier, member_ptr);
+ ((AsyncCallThreadData*) data)->call_successful = browser_functions.getproperty(instance, parent_ptr, member_identifier, member_ptr);
IcedTeaPluginUtilities::printNPVariant(*member_ptr);
if (((AsyncCallThreadData*) data)->call_successful)
{
- IcedTeaPluginUtilities::JSIDToString(member_ptr, &member_ptr_str);
+ createJavaObjectFromVariant(instance, *member_ptr, &member_ptr_str);
((AsyncCallThreadData*) data)->result.append(member_ptr_str);
+
}
((AsyncCallThreadData*) data)->result_ready = true;
@@ -831,8 +833,8 @@
std::string* script_str;
NPIdentifier script_identifier;
NPString script = NPString();
- NPVariant* eval_result = new NPVariant();
- std::string eval_result_ptr_str = std::string();
+ NPVariant* eval_variant = new NPVariant();
+ std::string eval_variant_str = std::string();
PLUGIN_DEBUG("_eval called\n");
@@ -854,13 +856,19 @@
PLUGIN_DEBUG("Evaluating: %s\n", script.UTF8Characters);
#endif
- ((AsyncCallThreadData*) data)->call_successful = browser_functions.evaluate(instance, window_ptr, &script, eval_result);
- IcedTeaPluginUtilities::printNPVariant(*eval_result);
+ ((AsyncCallThreadData*) data)->call_successful = browser_functions.evaluate(instance, window_ptr, &script, eval_variant);
+ IcedTeaPluginUtilities::printNPVariant(*eval_variant);
if (((AsyncCallThreadData*) data)->call_successful)
{
- IcedTeaPluginUtilities::JSIDToString(eval_result, &eval_result_ptr_str);
- ((AsyncCallThreadData*) data)->result.append(eval_result_ptr_str);
+ if (eval_variant)
+ {
+ createJavaObjectFromVariant(instance, *eval_variant, &eval_variant_str);
+ } else
+ {
+ eval_variant_str = "0";
+ }
+ ((AsyncCallThreadData*) data)->result.append(eval_variant_str);
}
((AsyncCallThreadData*) data)->result_ready = true;
@@ -904,7 +912,15 @@
if (((AsyncCallThreadData*) data)->call_successful)
{
- IcedTeaPluginUtilities::JSIDToString(call_result, &call_result_ptr_str);
+
+ if (call_result)
+ {
+ createJavaObjectFromVariant(instance, *call_result, &call_result_ptr_str);
+ } else
+ {
+ call_result_ptr_str = "0";
+ }
+
((AsyncCallThreadData*) data)->result.append(call_result_ptr_str);
}
More information about the distro-pkg-dev
mailing list