vm_start_mutex use in IcedTeaNPPlugin.cc looks fishy

[pcpa at mandriva.com.br]

Deepak Bhole escreveu:
> * pcpa at mandriva.com.br <pcpa at mandriva.com.br> [2012-05-23 12:46]:
>> Deepak Bhole escreveu:
>> > Hi Martin,
>> >
>> > * Martin Olsson <mnemo at minimum.se> [2012-05-23 11:44]:
>> >> The usage of "vm_start_mutex" in IcedTeaNPPlugin.cc looks buggy to me.
>> >> Won't each thread just create and lock it's own mutex, thereby voiding the
>> >> raison d'être of the mutex?
>> >>
>> >
>> > Yep. I just noticed this a couple of days ago myself when trying to
>> > investigate zombie VMs someone was seeing (though it was unrelated).
>>
>>   I have a patch for it in
>> http://icedtea.classpath.org/bugzilla/show_bug.cgi?id=599
>> I ended up adding another unrelated patch to the same bug report
>> by mistake, see the icedtea6-1.8.2-mutex_and_leak.patch
>> attachment. BTW, there is an issue with the bugzilla, that does
>> not allow to see text/plain in the browser.
>>
>> > Haven't gotten around to creating a patch for it, but please feel free
>> > to. If not, it is still on my list (though not critical since I have
>> > never seen a bug due to it).
>>
>>   I remember it corrected problems for me, with multiple instances
>> of jmol, but did not test it without the patch for a long time.
>>
>
> Thanks Paulo! I took a slightly different approach and have posted a
> patch for review here for this issue:
> http://mail.openjdk.java.net/pipermail/distro-pkg-dev/2012-May/018700.html

  You're welcome :-) Personally I do not like C++ code with static
variables initialized by a function call, but if it works, not much
of an issue, instead of checking and if NULL initializing it at
runtime.
  My original patch did not cover NP_shutdown, so it should have its
own problem in case the plugin is somehow unloaded or restarted.

> As for the second issue (msg_queue_mutex), the patch looks good to me. I
> will do some quick testing to ensure all is good and commit it on your
> behalf. I'll post a reply here when done.

  I remember I added it because there were conditions there would be no
listeners, and it would leak memory, but not sure if the condition of
no listeners was common, or side effect of some other problem.

  I have also the patch at
http://icedtea.classpath.org/bugzilla/show_bug.cgi?id=621
but I did not need it anymore since firefox-3.6.13 so the change of a
few types from int to long and atoi to atol should no longer be required
in x86_64. But may be worth checking as this may cause issues again.

> Thanks again,
> Deepak
>
>> > Cheers,
>> > Deepak
>>
>> Paulo

Paulo