Re: RFR(s): 8023541 Race condition in rmid initialization

2014-01-31 Thread Paul Sandoz
On Jan 30, 2014, at 11:02 PM, Stuart Marks stuart.ma...@oracle.com wrote: Maybe. I'd guess that the new JMM will stick to covering well-behaved programs (e.g. ones that adhere to safe publication). The difficulty with issues like this one is that once publication has occurred unsafely, we

Re: RFR(s): 8023541 Race condition in rmid initialization

2014-01-31 Thread Doug Lea
On 01/31/2014 08:32 AM, Paul Sandoz wrote: On Jan 30, 2014, at 11:02 PM, Stuart Marks stuart.ma...@oracle.com wrote: Maybe. I'd guess that the new JMM will stick to covering well-behaved programs (e.g. ones that adhere to safe publication). The difficulty with issues like this one is that

Re: RFR(s): 8023541 Race condition in rmid initialization

2014-01-30 Thread Daniel Fuchs
Hi Stuart, I wonder whether you should replace the assert in the constructor by an explicit null check: - assert systemStub != null + if (systemStub == null) throw new NullPointerException(); The reason I see is that before your change, an object constructed with a null systemStub would have

Re: RFR(s): 8023541 Race condition in rmid initialization

2014-01-30 Thread David Holmes
On 30/01/2014 5:34 PM, Tristan Yan wrote: Hi Stuart I didn’t make my comment clear. You set interrupted as true when thread gets interrupted. Here it's still going to wait until systemStub is not null. Why do you still need interrupt current thread in this case. Because the golden rule of

Re: RFR(s): 8023541 Race condition in rmid initialization

2014-01-30 Thread Paul Sandoz
On Jan 30, 2014, at 3:57 AM, Stuart Marks stuart.ma...@oracle.com wrote: Then, awaitInitialized seems straightforward until you see that the condition is waiting for the value of a final variable to change! JLS sec 17.5 [1] allows all sorts of optimizations for final fields, but they all

Re: RFR(s): 8023541 Race condition in rmid initialization

2014-01-30 Thread Alan Bateman
On 30/01/2014 09:09, Daniel Fuchs wrote: Hi Stuart, I wonder whether you should replace the assert in the constructor by an explicit null check: - assert systemStub != null + if (systemStub == null) throw new NullPointerException(); The reason I see is that before your change, an object

Re: RFR(s): 8023541 Race condition in rmid initialization

2014-01-30 Thread Stuart Marks
On 1/30/14 2:35 AM, David Holmes wrote: On 30/01/2014 5:34 PM, Tristan Yan wrote: Hi Stuart I didn’t make my comment clear. You set interrupted as true when thread gets interrupted. Here it's still going to wait until systemStub is not null. Why do you still need interrupt current thread in

Re: RFR(s): 8023541 Race condition in rmid initialization

2014-01-30 Thread Stuart Marks
On 1/30/14 1:09 AM, Daniel Fuchs wrote: I wonder whether you should replace the assert in the constructor by an explicit null check: - assert systemStub != null + if (systemStub == null) throw new NullPointerException(); The reason I see is that before your change, an object constructed with

Re: RFR(s): 8023541 Race condition in rmid initialization

2014-01-30 Thread Stuart Marks
On 1/30/14 3:13 AM, Paul Sandoz wrote: On Jan 30, 2014, at 3:57 AM, Stuart Marks stuart.ma...@oracle.com wrote: Then, awaitInitialized seems straightforward until you see that the condition is waiting for the value of a final variable to change! JLS sec 17.5 [1] allows all sorts of

Re: RFR(s): 8023541 Race condition in rmid initialization

2014-01-29 Thread David Holmes
Hi Stuart, This looks fine to me. Tristan: the initialized field is only accessed under synchronization so does not need to be volatile. Cheers, David On 29/01/2014 4:51 PM, Stuart Marks wrote: Hi all, Please review this fix to a race condition in rmid initialization. Briefly, rmid

Re: RFR(s): 8023541 Race condition in rmid initialization

2014-01-29 Thread Alan Bateman
On 29/01/2014 06:51, Stuart Marks wrote: Hi all, Please review this fix to a race condition in rmid initialization. Briefly, rmid subclasses the RMI registry implementation and provides special handling for its own stub. Unfortunately the registry is exported in the super() call, making

Re: RFR(s): 8023541 Race condition in rmid initialization

2014-01-29 Thread Paul Sandoz
On Jan 29, 2014, at 10:29 AM, Alan Bateman alan.bate...@oracle.com wrote: I just wonder if you could change initialized to volatile and only synchronize/wait when false? That way you would only be adding a read of a volatile. I assume the stub can never be null so maybe it could be changed

Re: RFR(s): 8023541 Race condition in rmid initialization

2014-01-29 Thread Peter Levart
Hi Stuart, My eye was caught by the following new method: 328 private synchronized void awaitInitialized() { 329 try { 330 while (!initialized) { 331 wait(); 332 } 333 } catch (InterruptedException ie) {

Re: RFR(s): 8023541 Race condition in rmid initialization

2014-01-29 Thread Stuart Marks
Hi all, wow, lots of comments on this. Let me see if I can tackle them in one message. Quick aside before I get to the issues: my priorities for this code are correctness and maintainability, possibly at the expense of performance. In other words I'm willing to make the code more complex so

Re: RFR(s): 8023541 Race condition in rmid initialization

2014-01-29 Thread David Holmes
This simpler form, with the interruption logic corrected, seems fine to me. David On 30/01/2014 12:57 PM, Stuart Marks wrote: Hi all, wow, lots of comments on this. Let me see if I can tackle them in one message. Quick aside before I get to the issues: my priorities for this code are

Re: RFR(s): 8023541 Race condition in rmid initialization

2014-01-29 Thread Tristan Yan
Hi Stuart Looks like in you new webrev. The wait will continue to go loop until systemStub is not null. If it’s what you meant to do that? Thank you Tristan On Jan 30, 2014, at 10:57 AM, Stuart Marks stuart.ma...@oracle.com wrote: Hi all, wow, lots of comments on this. Let me see if I can

Re: RFR(s): 8023541 Race condition in rmid initialization

2014-01-29 Thread Stuart Marks
On 1/29/14 8:50 PM, Tristan Yan wrote: Looks like in you new webrev. The wait will continue to go loop until systemStub is not null. If it’s what you meant to do that? Yes. In the previous webrev, systemStub started off as null and made a single transition to non-null. The boolean

Re: RFR(s): 8023541 Race condition in rmid initialization

2014-01-29 Thread Tristan Yan
Hi Stuart I didn’t make my comment clear. You set interrupted as true when thread gets interrupted. Here it's still going to wait until systemStub is not null. Why do you still need interrupt current thread in this case. Thank you Tristan On Jan 30, 2014, at 11:24 AM, David Holmes

RFR(s): 8023541 Race condition in rmid initialization

2014-01-28 Thread Stuart Marks
Hi all, Please review this fix to a race condition in rmid initialization. Briefly, rmid subclasses the RMI registry implementation and provides special handling for its own stub. Unfortunately the registry is exported in the super() call, making remote calls possible before rmid's stub

Re: RFR(s): 8023541 Race condition in rmid initialization

2014-01-28 Thread Tristan Yan
Hi Stuart Should variable initialized be volatile here? Otherwise looks good. Thank you Tristan On Jan 29, 2014, at 2:51 PM, Stuart Marks stuart.ma...@oracle.com wrote: Hi all, Please review this fix to a race condition in rmid initialization. Briefly, rmid subclasses the RMI registry