43090 - HP requires small change to oji-plugin interface to work

Status: VERIFIED FIXED

(Core Graveyard :: Java: OJI, defect, P3)

Description

[Jim Dunn]

modules/oji/src/nsJVMManager.cpp requires some 'monitor' protection
to make the OJI stuff threadsafe.

diff -r1.33 nsJVMManager.cpp
514a515,519
> #ifdef __hpux
>    EnterMonitor(&fStatus);
> #endif // __hpux
>
600a610,616
> #ifdef __hpux
>    ExitMonitor($fStatus);
> #endif // __hpux

or something like this..

Comment 1

[Jim Dunn]

file is at /u/jdunn/nsJVMManager.cpp

Comment 2

[Rob Jaworski]

adding jaworski to cc list.

Comment 3

[Jim Dunn]

reassiging to John

John we need a non HPUX specific way to do this.
Right now I am thinking of doing it like:
in configure have a REQUIRE_MUTEX_LOCKING_FOR_OJI
and set it for hpux.11 (we don't need it for 10.20)
then do an ifdef REQUIRE... in the JVMManager code

Beard or someone would have to review... but before
going down this path I would strongly suggest talking
with them, letting them know the problem and intended
solution.  You don't want to spend any time on this if
in the end they have a better idea or don't like the
solution.

Comment 4

[John Gaunt (redfive)]

accepting, adding shannon to cc

getting in touch with beard about this

Comment 5

[John Gaunt (redfive)]

Attachment: file containing updated changes to nsJVMManager.cpp

Comment 6

[John Gaunt (redfive)]

adding leaf to cc for review of configure additions. I believe Jim emailed you 
about the changes. if not, let him or i know and we will detail them for you.

the changes in the previously attached file are an expansion of changes suggesed 
by gcross of HP. jdunn has removed the reference to HP in the ifdef and has 
added releases of the locks around several return statements that were 
overlooked. We are currently waiting to hear from members of the build team 
regarding the addition of the configure variable jim used to enable the monitor 
locks. If adding the variable MOZ_OJI_REQUIRE_THREAD_SAFE_ON_STARTUP to the 
configure scripts is not a good idea ( or we need a better name ) please let us 
know a better alternative ( ie. makefile... )

Comment 7

[Daniel (Leaf) Nunes]

There a reason we aren't getting the regular OJI crew involved? edburns, beard,
etc.

I think adding stuff to configure is the right thing to do, or a config file.
Chris?

Comment 8

[cls]

Adding that define to configure as needed sounds good to me.

Comment 9

[John Gaunt (redfive)]

Thanks guys for the ok on the configure changes. I have been in contact with 
beard, gdrapeau and edburns already, beard has given the ok, and I'm still 
talking with drapeau and burns ( cc-ing george to the bug does Ed have a 
bugzilla acct? didn't let me add him )


adding George and Ed to CC list

Comment 10

[drapeau]

Adding Ed Burns to CC list.

Allow me to take this time to speak a tiny bit about Java development on Unix.
A group at Sun, led by David Therkelsen ([e-mail]) has recently taken
over the Sun Java Plug-In.  They have people hiring on to staff development of
the Unix ports.  ETA on that is a few weeks from now.  Meanwhile, a big merge of
several workspaces (Solaris latest plug-in code and Linux JDK workspace) is
happening right now, is expected to complete by the end of this week.  And last,
Nikolay Igotti, an engineer working for me on OJI, has been working on getting
Java Plug-In working on Linux.  He has it working, I have it on my Linux
notebook computer, and he will now merge in his changes into the Sun Plug-In
master workspace.  Due date for this: Monday, August 7.  If all goes well, Sun
should have Java Plug-In source for Unix ready to share with licensees again,
finally.  HP should be able to get that through its licensee contacts, and I
recommend HP contact Therkelsen and Stanley Ho ([e-mail]) ASAP for
details.

I have sent email to PDT asking about how to deliver Java bits for Linux for
internal testing.  I'd love to push this out for PR 2, but we've missed the
date.  PDT has been quite clear about drop dead dates; I've simply missed the
boat.

PR 3 is gonna be great, though.

Comment 11

[John Gaunt (redfive)]

ed, george, have you been able to look at the fix? since you are the module 
owner I need your approval to check this in. thanks - jg

Comment 12

[drapeau]

John: can you post the change as a diff, rather than the whole file?
I'll see you on Wednesday at the guest cube, we can talk more then.

Comment 13

[John Gaunt (redfive)]

Attachment: diff file of the changes in previously posted file.

Comment 14

[edburns]

These changes look fine to me.  All EnterMonitor() calls are paired with 
ExitMonitor() calls.  Also, as you'd expect, applying this patch to a windows 
build does not break OJI.  I tested it after applying the patch.

Ed

Comment 15

[drapeau]

Looks like my previous attempt at a bug comment got clobbered, so I'll put it in
again.
John: thank you for the diff.  I looked at it, with the full file attachment as
well.  I agree with Ed: it looks good.  Since Ed approves, I approve.

a=[e-mail].  Go for it, man.

Comment 16

[John Gaunt (redfive)]

fixed

Comment 17

[John Unruh]

Verified per gaunt's comments.