Re: Review Request JDK-8181834: Broken link in jdk.jdi module documentation

2017-06-19 Thread serguei.spit...@oracle.com 

Thumbs up.

Thanks,
Serguei

On 6/19/17 13:34, Mandy Chung wrote:

+ * 
On Jun 19, 2017, at 1:31 PM, serguei.spit...@oracle.com wrote:




On 6/19/17 13:23, Mandy Chung wrote:

On Jun 19, 2017, at 1:17 PM, serguei.spit...@oracle.com wrote:

Hi Mandy,

I've built the docs with your patch and the link is still not resolved.
It points to the docs/api/specs/jpda/jpda.html


It should point to docs/specs/jpda/jpda.html. I verified in my build.

Good. I'm Ok with the fix then.


Are you sure your build is correct?

No, I'm not sure.



But it is resolved if the patch is corrected with:
  + * 
I want to fix this simple docs bug in JDK 9.

Ok, thanks.

-Serguei


Mandy


Thanks,
Serguei


On 6/19/17 12:47, Mandy Chung wrote:

Serguei,

Can you review this patch fixing the link to JPDA?

Mandy



diff --git a/src/jdk.jdi/share/classes/module-info.java 
b/src/jdk.jdi/share/classes/module-info.java
--- a/src/jdk.jdi/share/classes/module-info.java
+++ b/src/jdk.jdi/share/classes/module-info.java
@@ -39,12 +39,13 @@
   * creation, etc. The ability to inspect a suspended thread's state, local
   * variables, stack backtrace, etc.
   * 
- * JDI is the highest-layer of the Java Platform Debugger Architecture (JPDA).
- * For more information on the Java Platform Debugger Architecture, see the  Java
- * Platform Debugger Architecture documentation for this release and the http://java.sun.com/products/jpda;>Java Platform Debugger Architecture
- * website.
+ * JDI is the highest-layer of the
+ * 
+ * Java Platform Debugger Architecture (JPDA).
+ * 
+ * This module includes a simple command-line debugger,
+ * {@index jdb jdb tool}.
+ *
   * Global Exceptions
   * 
   * This section documents exceptions which apply to the entire API and are 
thus
@@ -102,10 +103,6 @@
   *   unloaded.
   * 
   *
- * jdb
- *
- * {@index jdb jdb tool} is a simple command-line debugger provided
- * in this module.
   *
   * 
   * Tool Guides:
@@ -119,6 +116,8 @@
   *
   * @moduleGraph
   * @since 9
+ * @see 
+ * Java Platform Debugger Architecture (JPDA)
   */
  module jdk.jdi {
  requires jdk.attach;

Re: Review Request JDK-8181834: Broken link in jdk.jdi module documentation

2017-06-19 Thread Mandy Chung 
+ * 
>>> Is it 9 or 10?
>>> 
>> I want to fix this simple docs bug in JDK 9.
> 
> Ok, thanks.
> 
> -Serguei
> 
>> 
>> Mandy
>> 
>>> Thanks,
>>> Serguei
>>> 
>>> 
>>> On 6/19/17 12:47, Mandy Chung wrote:
 Serguei,
 
 Can you review this patch fixing the link to JPDA?
 
 Mandy
 
 
 
 diff --git a/src/jdk.jdi/share/classes/module-info.java 
 b/src/jdk.jdi/share/classes/module-info.java
 --- a/src/jdk.jdi/share/classes/module-info.java
 +++ b/src/jdk.jdi/share/classes/module-info.java
 @@ -39,12 +39,13 @@
   * creation, etc. The ability to inspect a suspended thread's state, local
   * variables, stack backtrace, etc.
   * 
 - * JDI is the highest-layer of the Java Platform Debugger Architecture 
 (JPDA).
 - * For more information on the Java Platform Debugger Architecture, see 
 the >>> - * href="{@docRoot}/../../../../technotes/guides/jpda/index.html"> Java
 - * Platform Debugger Architecture documentation for this release and 
 the >>> - * href="http://java.sun.com/products/jpda;>Java Platform Debugger 
 Architecture
 - * website.
 + * JDI is the highest-layer of the
 + * 
 + * Java Platform Debugger Architecture (JPDA).
 + * 
 + * This module includes a simple command-line debugger,
 + * {@index jdb jdb tool}.
 + *
   * Global Exceptions
   * 
   * This section documents exceptions which apply to the entire API and 
 are thus
 @@ -102,10 +103,6 @@
   *   unloaded.
   * 
   *
 - * jdb
 - *
 - * {@index jdb jdb tool} is a simple command-line debugger 
 provided
 - * in this module.
   *
   * 
   * Tool Guides:
 @@ -119,6 +116,8 @@
   *
   * @moduleGraph
   * @since 9
 + * @see 
 + * Java Platform Debugger Architecture (JPDA)
   */
  module jdk.jdi {
  requires jdk.attach;
 
>

Re: Review Request JDK-8181834: Broken link in jdk.jdi module documentation

2017-06-19 Thread serguei.spit...@oracle.com 




On 6/19/17 13:23, Mandy Chung wrote:

On Jun 19, 2017, at 1:17 PM, serguei.spit...@oracle.com wrote:

Hi Mandy,

I've built the docs with your patch and the link is still not resolved.
It points to the docs/api/specs/jpda/jpda.html


It should point to docs/specs/jpda/jpda.html. I verified in my build.


Good. I'm Ok with the fix then.


Are you sure your build is correct?


No, I'm not sure.



But it is resolved if the patch is corrected with:
  + * 
I want to fix this simple docs bug in JDK 9.


Ok, thanks.

-Serguei



Mandy


Thanks,
Serguei


On 6/19/17 12:47, Mandy Chung wrote:

Serguei,

Can you review this patch fixing the link to JPDA?

Mandy



diff --git a/src/jdk.jdi/share/classes/module-info.java 
b/src/jdk.jdi/share/classes/module-info.java
--- a/src/jdk.jdi/share/classes/module-info.java
+++ b/src/jdk.jdi/share/classes/module-info.java
@@ -39,12 +39,13 @@
   * creation, etc. The ability to inspect a suspended thread's state, local
   * variables, stack backtrace, etc.
   * 
- * JDI is the highest-layer of the Java Platform Debugger Architecture (JPDA).
- * For more information on the Java Platform Debugger Architecture, see the  Java
- * Platform Debugger Architecture documentation for this release and the http://java.sun.com/products/jpda;>Java Platform Debugger Architecture
- * website.
+ * JDI is the highest-layer of the
+ * 
+ * Java Platform Debugger Architecture (JPDA).
+ * 
+ * This module includes a simple command-line debugger,
+ * {@index jdb jdb tool}.
+ *
   * Global Exceptions
   * 
   * This section documents exceptions which apply to the entire API and are 
thus
@@ -102,10 +103,6 @@
   *   unloaded.
   * 
   *
- * jdb
- *
- * {@index jdb jdb tool} is a simple command-line debugger provided
- * in this module.
   *
   * 
   * Tool Guides:
@@ -119,6 +116,8 @@
   *
   * @moduleGraph
   * @since 9
+ * @see 
+ * Java Platform Debugger Architecture (JPDA)
   */
  module jdk.jdi {
  requires jdk.attach;

Re: Review Request JDK-8181834: Broken link in jdk.jdi module documentation

2017-06-19 Thread serguei.spit...@oracle.com 

Hi Mandy,

I've built the docs with your patch and the link is still not resolved.
It points to the docs/api/specs/jpda/jpda.html

But it is resolved if the patch is corrected with:
  + * 


Is it 9 or 10?

Thanks,
Serguei


On 6/19/17 12:47, Mandy Chung wrote:

Serguei,

Can you review this patch fixing the link to JPDA?

Mandy



diff --git a/src/jdk.jdi/share/classes/module-info.java 
b/src/jdk.jdi/share/classes/module-info.java
--- a/src/jdk.jdi/share/classes/module-info.java
+++ b/src/jdk.jdi/share/classes/module-info.java
@@ -39,12 +39,13 @@
   * creation, etc. The ability to inspect a suspended thread's state, local
   * variables, stack backtrace, etc.
   * 
- * JDI is the highest-layer of the Java Platform Debugger Architecture (JPDA).
- * For more information on the Java Platform Debugger Architecture, see the  Java
- * Platform Debugger Architecture documentation for this release and the http://java.sun.com/products/jpda;>Java Platform Debugger Architecture
- * website.
+ * JDI is the highest-layer of the
+ * 
+ * Java Platform Debugger Architecture (JPDA).
+ * 
+ * This module includes a simple command-line debugger,
+ * {@index jdb jdb tool}.
+ *
   * Global Exceptions
   * 
   * This section documents exceptions which apply to the entire API and are 
thus
@@ -102,10 +103,6 @@
   *   unloaded.
   * 
   *
- * jdb
- *
- * {@index jdb jdb tool} is a simple command-line debugger provided
- * in this module.
   *
   * 
   * Tool Guides:
@@ -119,6 +116,8 @@
   *
   * @moduleGraph
   * @since 9
+ * @see 
+ * Java Platform Debugger Architecture (JPDA)
   */
  module jdk.jdi {
  requires jdk.attach;

Review Request JDK-8181834: Broken link in jdk.jdi module documentation

2017-06-19 Thread Mandy Chung 
Serguei,

Can you review this patch fixing the link to JPDA?

Mandy



diff --git a/src/jdk.jdi/share/classes/module-info.java 
b/src/jdk.jdi/share/classes/module-info.java
--- a/src/jdk.jdi/share/classes/module-info.java
+++ b/src/jdk.jdi/share/classes/module-info.java
@@ -39,12 +39,13 @@
  * creation, etc. The ability to inspect a suspended thread's state, local
  * variables, stack backtrace, etc.
  * 
- * JDI is the highest-layer of the Java Platform Debugger Architecture (JPDA).
- * For more information on the Java Platform Debugger Architecture, see the  Java
- * Platform Debugger Architecture documentation for this release and the http://java.sun.com/products/jpda;>Java Platform Debugger Architecture
- * website.
+ * JDI is the highest-layer of the 
+ * 
+ * Java Platform Debugger Architecture (JPDA).
+ * 
+ * This module includes a simple command-line debugger,
+ * {@index jdb jdb tool}.
+ *
  * Global Exceptions
  * 
  * This section documents exceptions which apply to the entire API and are thus
@@ -102,10 +103,6 @@
  *   unloaded.
  * 
  *
- * jdb
- *
- * {@index jdb jdb tool} is a simple command-line debugger provided
- * in this module.
  *
  * 
  * Tool Guides:
@@ -119,6 +116,8 @@
  *
  * @moduleGraph
  * @since 9
+ * @see 
+ * Java Platform Debugger Architecture (JPDA)
  */
 module jdk.jdi {
 requires jdk.attach;

Re: RFR: JDK-8173180 VirtualMachine.startLocalManagementAgent() returns URI with unreliable IP address

2017-06-19 Thread Daniel Fuchs 

Hi,

If I'm not mistaken then this will make it impossible
for earlier release to interoperate with newer releases
as the LocalRMIClientSocketFactory class will not be
present the client tries to deserialize the stub.

best regards,

-- daniel

On 19/06/2017 11:52, Ujwal Vangapally wrote:

Hi,

Kindly review the fix for bug below

https://bugs.openjdk.java.net/browse/JDK-8173180

webrev: 
http://cr.openjdk.java.net/~uvangapally/webrev/2017/8173180/webrev.00/


Thanks,

Ujwal

RFR: JDK-8173180 VirtualMachine.startLocalManagementAgent() returns URI with unreliable IP address

2017-06-19 Thread Ujwal Vangapally 

Hi,

Kindly review the fix for bug below

https://bugs.openjdk.java.net/browse/JDK-8173180

webrev: 
http://cr.openjdk.java.net/~uvangapally/webrev/2017/8173180/webrev.00/


Thanks,

Ujwal

Re: RFR(xs): 8181419: Race in jdwp invoker handling may lead to crashes or invalid results

2017-06-19 Thread Thomas Stüfe 
Thank you Christoph!

On Mon, Jun 19, 2017 at 11:31 AM, Langer, Christoph <
christoph.lan...@sap.com> wrote:

> Hi Thomas,
>
>
>
> this looks good to me. You still need to update the current year in the
> copyright header.
>
>
>
> Best regards
>
> Christoph
>
>
>
> *From:* serviceability-dev [mailto:serviceability-dev-
> boun...@openjdk.java.net] *On Behalf Of *Thomas Stüfe
> *Sent:* Donnerstag, 15. Juni 2017 08:31
> *To:* serviceability-dev@openjdk.java.net
> *Subject:* Re: RFR(xs): 8181419: Race in jdwp invoker handling may lead
> to crashes or invalid results
>
>
>
> Hi Folks,
>
>
>
> may I please have a second reviewer?
>
>
>
> Thank you!
>
>
>
> Kind Regards, Thomas
>
>
>
> On Tue, Jun 13, 2017 at 11:16 PM, serguei.spit...@oracle.com <
> serguei.spit...@oracle.com> wrote:
>
> Hi Thomas,
>
> The fix looks great to me.
> Thank you for identifying and fixing the problem!
>
> Thanks,
> Serguei
>
>
>
>
> On 6/13/17 06:55, Thomas Stüfe wrote:
>
> Ping... Anyone?
>
>
>
> On Thu, Jun 1, 2017 at 2:18 PM, Thomas Stüfe 
> wrote:
>
> Hi all,
>
>
>
> please take a look at this proposed fix for a theoretical race in the jdwp
> library.
>
>
>
> Issue: https://bugs.openjdk.java.net/browse/JDK-8181419
>
> webrev: http://cr.openjdk.java.net/~stuefe/webrevs/
> 8181419-Race-in-jdwp-invoker-handling-may-lead-to-crashes-
> or-invalid-results/webrev.00/webrev/
>
>
>
> In short, this is an addition to Severin's fix to the jdwp invoke handling
> (https://bugs.openjdk.java.net/browse/JDK-8153711).
>
>
>
> We have a potential race condition where the delayed cleanup of the saved
> returnvalue object reference and the exception reference (released in
> deletePotentiallySavedGlobalRefs() ) may be overtaken by a new request
> which populates the thread request structure anew. If this happens,
> deletePotentiallySavedGlobalRefs() may actually release the return value
> / exception references of the follow up request, if that one was already
> processed.
>
>
>
> The solution I choose is safe and conservative. We still release both
> references, but use the locally saved JNI references. We just avoid
> accessing the thread local request structure after it has been cleared for
> reuse. This keeps timing and locking behaviour unchanged.
>
>
>
> I am currently running jtreg tests for com/sun/jdi on AIX and Linux.
>
>
>
> Kind Regards, Thomas
>
>
>
>
>
>
>

RE: RFR(xs): 8181419: Race in jdwp invoker handling may lead to crashes or invalid results

2017-06-19 Thread Langer, Christoph 
Hi Thomas,

this looks good to me. You still need to update the current year in the 
copyright header.

Best regards
Christoph

From: serviceability-dev [mailto:serviceability-dev-boun...@openjdk.java.net] 
On Behalf Of Thomas Stüfe
Sent: Donnerstag, 15. Juni 2017 08:31
To: serviceability-dev@openjdk.java.net
Subject: Re: RFR(xs): 8181419: Race in jdwp invoker handling may lead to 
crashes or invalid results

Hi Folks,

may I please have a second reviewer?

Thank you!

Kind Regards, Thomas

On Tue, Jun 13, 2017 at 11:16 PM, 
serguei.spit...@oracle.com 
> wrote:
Hi Thomas,

The fix looks great to me.
Thank you for identifying and fixing the problem!

Thanks,
Serguei



On 6/13/17 06:55, Thomas Stüfe wrote:
Ping... Anyone?

On Thu, Jun 1, 2017 at 2:18 PM, Thomas Stüfe 
> wrote:
Hi all,

please take a look at this proposed fix for a theoretical race in the jdwp 
library.

Issue: https://bugs.openjdk.java.net/browse/JDK-8181419
webrev: 
http://cr.openjdk.java.net/~stuefe/webrevs/8181419-Race-in-jdwp-invoker-handling-may-lead-to-crashes-or-invalid-results/webrev.00/webrev/

In short, this is an addition to Severin's fix to the jdwp invoke handling 
(https://bugs.openjdk.java.net/browse/JDK-8153711).

We have a potential race condition where the delayed cleanup of the saved 
returnvalue object reference and the exception reference (released in 
deletePotentiallySavedGlobalRefs() ) may be overtaken by a new request which 
populates the thread request structure anew. If this happens, 
deletePotentiallySavedGlobalRefs() may actually release the return value / 
exception references of the follow up request, if that one was already 
processed.

The solution I choose is safe and conservative. We still release both 
references, but use the locally saved JNI references. We just avoid accessing 
the thread local request structure after it has been cleared for reuse. This 
keeps timing and locking behaviour unchanged.

I am currently running jtreg tests for com/sun/jdi on AIX and Linux.

Kind Regards, Thomas

Re: RFR(xs): 8181419: Race in jdwp invoker handling may lead to crashes or invalid results

2017-06-19 Thread Thomas Stüfe 
Thank you Severin!

On Mon, Jun 19, 2017 at 11:10 AM, Severin Gehwolf 
wrote:

> Hi Thomas,
>
> On Tue, 2017-06-13 at 15:55 +0200, Thomas Stüfe wrote:
> > Ping... Anyone?
> >
> > On Thu, Jun 1, 2017 at 2:18 PM, Thomas Stüfe  > > wrote:
> > > Hi all,
> > >
> > > please take a look at this proposed fix for a theoretical race in
> > > the jdwp library.
> > >
> > > Issue: https://bugs.openjdk.java.net/browse/JDK-8181419
> > > webrev: http://cr.openjdk.java.net/~stuefe/webrevs/
> 8181419-Race-in-jdwp-invoker-handling-may-lead-to-crashes-
> or-invalid-results/webrev.00/webrev/
> > >
> > > In short, this is an addition to Severin's fix to the jdwp invoke
> > > handling (https://bugs.openjdk.java.net/browse/JDK-8153711).
> > >
> > > We have a potential race condition where the delayed cleanup of the
> > > saved returnvalue object reference and the exception reference
> > > (released in deletePotentiallySavedGlobalRefs() ) may be overtaken
> > > by a new request which populates the thread request structure anew.
> > > If this happens, deletePotentiallySavedGlobalRefs() may actually
> > > release the return value / exception references of the follow up
> > > request, if that one was already processed.
> > >
> > > The solution I choose is safe and conservative. We still release
> > > both references, but use the locally saved JNI references. We just
> > > avoid accessing the thread local request structure after it has
> > > been cleared for reuse. This keeps timing and locking behaviour
> > > unchanged.
> > >
> > > I am currently running jtreg tests for com/sun/jdi on AIX and
> > > Linux.
>
> The fix makes sense to me. Looks good! I'm not an OpenJDK Reviewer.
>
> Cheers,
> Severin
>

Re: RFR(xs): 8181419: Race in jdwp invoker handling may lead to crashes or invalid results

2017-06-19 Thread Severin Gehwolf 
Hi Thomas,

On Tue, 2017-06-13 at 15:55 +0200, Thomas Stüfe wrote:
> Ping... Anyone?
> 
> On Thu, Jun 1, 2017 at 2:18 PM, Thomas Stüfe  > wrote:
> > Hi all,
> > 
> > please take a look at this proposed fix for a theoretical race in
> > the jdwp library.
> > 
> > Issue: https://bugs.openjdk.java.net/browse/JDK-8181419
> > webrev: 
> > http://cr.openjdk.java.net/~stuefe/webrevs/8181419-Race-in-jdwp-invoker-handling-may-lead-to-crashes-or-invalid-results/webrev.00/webrev/
> > 
> > In short, this is an addition to Severin's fix to the jdwp invoke
> > handling (https://bugs.openjdk.java.net/browse/JDK-8153711).
> > 
> > We have a potential race condition where the delayed cleanup of the
> > saved returnvalue object reference and the exception reference
> > (released in deletePotentiallySavedGlobalRefs() ) may be overtaken
> > by a new request which populates the thread request structure anew.
> > If this happens, deletePotentiallySavedGlobalRefs() may actually
> > release the return value / exception references of the follow up
> > request, if that one was already processed.
> > 
> > The solution I choose is safe and conservative. We still release
> > both references, but use the locally saved JNI references. We just
> > avoid accessing the thread local request structure after it has
> > been cleared for reuse. This keeps timing and locking behaviour
> > unchanged.
> > 
> > I am currently running jtreg tests for com/sun/jdi on AIX and
> > Linux.

The fix makes sense to me. Looks good! I'm not an OpenJDK Reviewer.

Cheers,
Severin