date:20190701

Re: (13) RFR(XS): 8226596: Accessibility errors in jdwp-protocol.html

2019-07-01 Thread serguei.spit...@oracle.com


Hi David,


On 7/1/19 19:03, David Holmes wrote:

Hi Serguei,

The updates to the heading levels all looks fine.


Okay, thanks!




However the tables in the generated output look broken - they all have 
empty "header" rows with incorrect columns and no text. ??


I'm not sure about this.
What exact tables?
I do not see anything like this.


Please, see the diff below:

% diff 0/jdwp-protocol.html 1/jdwp-protocol.html
13c13,17
< 
---
> 
> Java Debug Wire Protocol Details
> 
> 
> 
171a176
> 
173c178
< VirtualMachine Command Set (1)
---
> VirtualMachine Command Set (1)
175c180
< Version Command (1)
---
> Version Command (1)
205c210
< ClassesBySignature 
Command (2)

---
> ClassesBySignature 
Command (2)

244c249
< AllClasses Command (3)
---
> AllClasses Command (3)
281c286
< AllThreads Command (4)
---
> AllThreads Command (4)
306c311
< TopLevelThreadGroups 
Command (5)

---
> id="JDWP_VirtualMachine_TopLevelThreadGroups">TopLevelThreadGroups 
Command (5)

331c336
< Dispose Command (6)
---
> Dispose Command (6)
341c346
< IDSizes Command (7)
---
> IDSizes Command (7)
371c376
< Suspend Command (8)
---
> Suspend Command (8)
383c388
< Resume Command (9)
---
> Resume Command (9)
393c398
< Exit Command (10)
---
> Exit Command (10)
409c414
< CreateString Command (11)
---
> CreateString Command (11)
433c438
< Capabilities Command (12)
---
> Capabilities Command (12)
469c474
< ClassPaths Command (13)
---
> ClassPaths Command (13)
507c512
< DisposeObjects Command 
(14)

---
> DisposeObjects Command 
(14)

534c539
< HoldEvents Command (15)
---
> HoldEvents Command (15)
544c549
< ReleaseEvents Command (16)
---
> ReleaseEvents Command 
(16)

554c559
< CapabilitiesNew Command 
(17)

---
> CapabilitiesNew Command 
(17)

665c670
< RedefineClasses Command 
(18)

---
> RedefineClasses Command 
(18)

717c722
< SetDefaultStratum 
Command (19)

---
> SetDefaultStratum 
Command (19)

736c741
< id="JDWP_VirtualMachine_AllClassesWithGeneric">AllClassesWithGeneric 
Command (20)

---
> id="JDWP_VirtualMachine_AllClassesWithGeneric">AllClassesWithGeneric 
Command (20)

777c782
< InstanceCounts Command 
(21)

---
> InstanceCounts Command 
(21)

817c822
< AllModules Command (22)
---
> AllModules Command (22)
843c848
< ReferenceType Command Set (2)
---
> ReferenceType Command Set (2)
845c850
< Signature Command (1)
---
> Signature Command (1)
871c876
< ClassLoader Command (2)
---
> ClassLoader Command (2)
897c902
< Modifiers Command (3)
---
> Modifiers Command (3)
923c928
< Fields Command (4)
---
> Fields Command (4)
969c974
< Methods Command (5)
---
> Methods Command (5)
1015c1020
< GetValues Command (6)
---
> GetValues Command (6)
1059c1064
< SourceFile Command (7)
---
> SourceFile Command (7)
1086c1091
< NestedTypes Command (8)
---
> NestedTypes Command (8)
1123c1128
< Status Command (9)
---
> Status Command (9)
1149c1154
< Interfaces Command (10)
---
> Interfaces Command (10)
1182c1187
< ClassObject Command (11)
---
> ClassObject Command (11)
1208c1213
< SourceDebugExtension 
Command (12)

---
> SourceDebugExtension 
Command (12)

1236c1241
< SignatureWithGeneric 
Command (13)

---
> SignatureWithGeneric 
Command (13)

1265c1270
< FieldsWithGeneric 
Command (14)

---
> FieldsWithGeneric 
Command (14)

1315c1320
< MethodsWithGeneric 
Command (15)

---
> MethodsWithGeneric 
Command (15)

1365c1370
< Instances Command (16)
---
> Instances Command (16)
1403c1408
< ClassFileVersion Command 
(17)

---
> ClassFileVersion Command 
(17)

1433c1438
< ConstantPool Command (18)
---
> ConstantPool Command (18)
1471c1476
< Module Command (19)
---
> Module Command (19)
1498c1503
< ClassType Command Set (3)
---
> ClassType Command Set (3)
1500c1505
< Superclass Command (1)
---
> Superclass Command (1)
1526c1531
< SetValues Command (2)
---
> SetValues Command (2)
1562c1567
< InvokeMethod Command (3)
---
> InvokeMethod Command (3)
1613c1618
< NewInstance Command (4)
---
> NewInstance Command (4)
1665c1670
< ArrayType Command Set (4)
---
> ArrayType Command Set (4)
1667c1672
< NewInstance Command (1)
---
> NewInstance Command (1)
1696c1701
< InterfaceType Command Set (5)
---
> InterfaceType Command Set (5)
1698c1703
< InvokeMethod Command (1)
---
> InvokeMethod Command (1)
1749c1754
< Method Command Set (6)
---
> Method Command Set (6)
1751c1756
< LineTable Command (1)
---
> LineTable Command (1)
1798c1803
< VariableTable Command (2)
---
> VariableTable Command (2)
1855c1860
< Bytecodes Command (3)
---
> Bytecodes Command (3)
1893c1898
< IsObsolete Command (4)
---
> IsObsolete Command (4)
1924c1929
< VariableTableWithGeneric 
Command (5)

---
> id="JDWP_Method_VariableTableWithGeneric">VariableTableWithGeneric 
Command (5)

1985c1990
< Field Command Set (8)
---
> Field Command Set (8)
1987c1992
< ObjectReference Command Set (9)
---
> ObjectReference Command Set (9)
1989c1994
< ReferenceType Command (1)
---
> ReferenceType Command 
(1)

2017c2022
< GetValues Command (2)
---
> GetValues Command (2)
2060c2065
< SetValues Command (3)
---
> SetValues

Re: (13) RFR(XS): 8226596: Accessibility errors in jdwp-protocol.html

2019-07-01 Thread David Holmes


Hi Serguei,

The updates to the heading levels all looks fine.

However the tables in the generated output look broken - they all have 
empty "header" rows with incorrect columns and no text. ??


David

On 2/07/2019 11:08 am, serguei.spit...@oracle.com wrote:

Hi David,


On 7/1/19 14:15, David Holmes wrote:

Hi Serguei,

On 2/07/2019 4:57 am, serguei.spit...@oracle.com wrote:

Please, review a doc issue fix for:
https://bugs.openjdk.java.net/browse/JDK-8226596

Webrev:
http://cr.openjdk.java.net/~sspitsyn/webrevs/2019/8226596-jdwp-a11y.1/

The updated JDWP protocol page:
http://cr.openjdk.java.net/~sspitsyn/webrevs/2019/jdwp-protocol.html


Summary:
   The change is to replace the 'navigation' attribute with the  
element.


That looks fine.


   With this fix, the Axe reports one issue only:
 "Page must contain a level one heading".

I'm not sure what to do to workaround this.
The page was not initially designed to have a level one heading.
It has to be normally used in the context of enclosed page which 
haswith a level one heading.


Not sure what you mean by an "enclosed page". Where can I see this in 
context?


Sorry, I wanted to say "enclosing page".

We had a private chat with David on this.
The decision is to add a heading one to the page and align all other 
headings with it.


New webrev:
http://cr.openjdk.java.net/~sspitsyn/webrevs/2019/8226596-jdwp-a11y.2/

I've updated the file:
http://cr.openjdk.java.net/~sspitsyn/webrevs/2019/jdwp-protocol.html

The Axe does not report errors anymore.

Thanks,
Serguei



Thanks,
David
-


Testing:
   Verified with the Axe plugin (add-on) for Mozilla Firefox browser.
   Asked Alex M. to double-check this, and his Axe reported no errors.

Thanks,
Serguei

Re: (13) RFR(XS): 8226596: Accessibility errors in jdwp-protocol.html

2019-07-01 Thread serguei.spit...@oracle.com


Hi Alex,

Thank you for the review!
Could also look at the webrev v2? :
http://cr.openjdk.java.net/~sspitsyn/webrevs/2019/8226596-jdwp-a11y.2/

This change also updates page headings (result of our discussion with 
David H.).


Thanks,
Serguei


On 7/1/19 18:02, Alex Menkov wrote:

LGTM

--alex

On 07/01/2019 14:15, David Holmes wrote:

Hi Serguei,

On 2/07/2019 4:57 am, serguei.spit...@oracle.com wrote:

Please, review a doc issue fix for:
https://bugs.openjdk.java.net/browse/JDK-8226596

Webrev:
http://cr.openjdk.java.net/~sspitsyn/webrevs/2019/8226596-jdwp-a11y.1/

The updated JDWP protocol page:
http://cr.openjdk.java.net/~sspitsyn/webrevs/2019/jdwp-protocol.html


Summary:
   The change is to replace the 'navigation' attribute with the 
 element.


That looks fine.


   With this fix, the Axe reports one issue only:
 "Page must contain a level one heading".

I'm not sure what to do to workaround this.
The page was not initially designed to have a level one heading.
It has to be normally used in the context of enclosed page which 
haswith a level one heading.


Not sure what you mean by an "enclosed page". Where can I see this in 
context?


Thanks,
David
-


Testing:
   Verified with the Axe plugin (add-on) for Mozilla Firefox browser.
   Asked Alex M. to double-check this, and his Axe reported no errors.

Thanks,
Serguei

Re: (13) RFR(XS): 8226596: Accessibility errors in jdwp-protocol.html

2019-07-01 Thread serguei.spit...@oracle.com


Hi David,


On 7/1/19 14:15, David Holmes wrote:

Hi Serguei,

On 2/07/2019 4:57 am, serguei.spit...@oracle.com wrote:

Please, review a doc issue fix for:
https://bugs.openjdk.java.net/browse/JDK-8226596

Webrev:
http://cr.openjdk.java.net/~sspitsyn/webrevs/2019/8226596-jdwp-a11y.1/

The updated JDWP protocol page:
http://cr.openjdk.java.net/~sspitsyn/webrevs/2019/jdwp-protocol.html


Summary:
   The change is to replace the 'navigation' attribute with the  
element.


That looks fine.


   With this fix, the Axe reports one issue only:
 "Page must contain a level one heading".

I'm not sure what to do to workaround this.
The page was not initially designed to have a level one heading.
It has to be normally used in the context of enclosed page which 
haswith a level one heading.


Not sure what you mean by an "enclosed page". Where can I see this in 
context?


Sorry, I wanted to say "enclosing page".

We had a private chat with David on this.
The decision is to add a heading one to the page and align all other 
headings with it.


New webrev:
http://cr.openjdk.java.net/~sspitsyn/webrevs/2019/8226596-jdwp-a11y.2/

I've updated the file:
http://cr.openjdk.java.net/~sspitsyn/webrevs/2019/jdwp-protocol.html

The Axe does not report errors anymore.

Thanks,
Serguei



Thanks,
David
-


Testing:
   Verified with the Axe plugin (add-on) for Mozilla Firefox browser.
   Asked Alex M. to double-check this, and his Axe reported no errors.

Thanks,
Serguei

Re: (13) RFR(XS): 8226596: Accessibility errors in jdwp-protocol.html

2019-07-01 Thread Alex Menkov


LGTM

--alex

On 07/01/2019 14:15, David Holmes wrote:

Hi Serguei,

On 2/07/2019 4:57 am, serguei.spit...@oracle.com wrote:

Please, review a doc issue fix for:
https://bugs.openjdk.java.net/browse/JDK-8226596

Webrev:
http://cr.openjdk.java.net/~sspitsyn/webrevs/2019/8226596-jdwp-a11y.1/

The updated JDWP protocol page:
http://cr.openjdk.java.net/~sspitsyn/webrevs/2019/jdwp-protocol.html


Summary:
   The change is to replace the 'navigation' attribute with the  
element.


That looks fine.


   With this fix, the Axe reports one issue only:
 "Page must contain a level one heading".

I'm not sure what to do to workaround this.
The page was not initially designed to have a level one heading.
It has to be normally used in the context of enclosed page which 
haswith a level one heading.


Not sure what you mean by an "enclosed page". Where can I see this in 
context?


Thanks,
David
-


Testing:
   Verified with the Axe plugin (add-on) for Mozilla Firefox browser.
   Asked Alex M. to double-check this, and his Axe reported no errors.

Thanks,
Serguei

PING: RFR: 8209790: SA tools not providing option to connect to debug server

2019-07-01 Thread Yasumasa Suenaga

PING: Could you review it?

   http://cr.openjdk.java.net/~ysuenaga/JDK-8209790/webrev.03/

Yasumasa

On 2019/06/25 16:47, Yasumasa Suenaga wrote:

Hi,

This enhancement has been retargeted to 14, and the CSR has been approved.
I uploaded a webrev for 14. Could you review it?

   http://cr.openjdk.java.net/~ysuenaga/JDK-8209790/webrev.03/

It includes the fix to rename `--remote` to `--connect` that is
suggested in CSR.
It passed tests on submit repo.

Thanks,

Yasumasa

2019年6月17日(月) 22:11 Yasumasa Suenaga :

Hi David,

On 2019/06/17 21:42, David Holmes wrote:

Hi Yasumasa,

On 17/06/2019 6:50 pm, Yasumasa Suenaga wrote:

Hi David,

8209790 is filed as a bug.

I don't agree this is a "bug" - sorry. For this to be a bug there must
be some specification of behaviour that the implementation is violating.
Is that the case? To me this is missing functionality which makes it an
enhancement.

The feature for connecting to remote debug server has been provided JDK
8 or earlier. However it was missed since JDK 9. So I think we can
handle it as a "bug".
Anyway, I added jdk13-enhancement-request label to JBS. I'm waiting for
the approval.

According to [1], I think we can push the fix to jdk/jdk13. Does it
correct?

I'm not sure which version (13 or 14) should be set on JBS before
pushing.

Just to clarify the process here, you don't want to set the "Fix
Version" to 13 or 14 in JBS before pushing. That will be set based on
the repo you push to. If you push to jdk/jdk it will be 14. If you push
to jdk/jdk13 it will be 13. Any fix pushed to jdk/jdk13 will be
"automatically" forward-ported to jdk/jdk and thus 14.

Thanks! I got it.

Yasumasa

David
-

(Of course I will push it after the CSR is approved.)

Thanks,

Yasumasa

[1] http://mail.openjdk.java.net/pipermail/jdk-dev/2019-June/003051.html

On 2019/06/17 17:06, David Holmes wrote:

Hi Yasumasa,

On 17/06/2019 8:52 am, Yasumasa Suenaga wrote:

2019年6月17日(月) 6:47 serguei.spit...@oracle.com
 mailto:serguei.spit...@oracle.com>>:

 Forgot to tell...
 This can be pushed only after the CSR is approved.

Sure!
And I will push it when I get second reviewer.

BTW should I push this change to jdk/jdk? or push to jdk/jdk13
manually?

JDK 13 has now entered RDP1 so if you want this to go into 13 it will
need special approval:

http://openjdk.java.net/jeps/3#Late-Enhancement-Request-Process

Otherwise push to jdk/jdk and it will be for JDK 14.

Cheers,
David

Thanks,

Yasumasa

 Thanks,
 Serguei

 On 6/16/19 14:44, serguei.spit...@oracle.com
  wrote:
  > Hi Yasumasa,
  >
  >
  > On 6/16/19 07:22, Yasumasa Suenaga wrote:
  >> Hi Serguei,
  >>
  >> >>> One minor suggestion is to use the final field NO_REMOTE
instead
  >> >>> of null for initialization of the local variable "remote".
  >>
  >> I fixed it on new webrev. Could you check again?
  >> http://cr.openjdk.java.net/~ysuenaga/JDK-8209790/webrev.02/
  >
  >
  > It looks good.
  > Thanks you for the update!
  >
  >>
  >> >> IMHO refactoring should be worked on another issue.
  >> >
  >> > Agreed.
  >>
  >> I filed it to JBS:
  >> https://bugs.openjdk.java.net/browse/JDK-8226204
  >
  > Thank you for filing the enhancement!
  >
  > Thanks.
  > Serguei
  >
  >
  >> Thanks,
  >>
  >> Yasumasa
  >>
  >>
  >> On 2019/06/15 15:10, serguei.spit...@oracle.com
  wrote:
  >>> Hi Yasumasa,
  >>>
  >>>
  >>> On 6/14/19 21:11, Yasumasa Suenaga wrote:
   Hi Serguei,

   Thank you for your comment!

   On 2019/06/15 8:00, serguei.spit...@oracle.com
  wrote:
  > Hi Yasumasa,
  >
  > I've added myself as a reviewer, so you can finalize it now.

   I moved CSR to Finalized, and added a comment for your
question.
  >>>
  >>> Okay, thanks!
  >>>
  >>>
  > The fix looks pretty good to me.
  >
  > One minor suggestion is to use the final field NO_REMOTE
instead
  > of null for initialization of the local variable "remote".

   I will fix that.

  > Also just an observation that there is some room for option
  > processing refactoring.

   Your suggestion handles all options in one parser method.
   I concern it might be complex for option validation.
   (e.g. `jmap -heap` is allowed, but `jstack -heap` is not
allowed.)
  >>>
  >>> This concern is not valid as the list allowed options allowed
 for each
  >>> jhsdb sub-command is controlled with the longOpts argument.
  >>>
  >>> jmap has:
  >>>   String[] longOpts = {"exe=", "core=",

Re: (13) RFR(XS): 8226596: Accessibility errors in jdwp-protocol.html

2019-07-01 Thread David Holmes


Hi Serguei,

On 2/07/2019 4:57 am, serguei.spit...@oracle.com wrote:

Please, review a doc issue fix for:
https://bugs.openjdk.java.net/browse/JDK-8226596

Webrev:
http://cr.openjdk.java.net/~sspitsyn/webrevs/2019/8226596-jdwp-a11y.1/

The updated JDWP protocol page:
http://cr.openjdk.java.net/~sspitsyn/webrevs/2019/jdwp-protocol.html


Summary:
   The change is to replace the 'navigation' attribute with the  
element.


That looks fine.


   With this fix, the Axe reports one issue only:
     "Page must contain a level one heading".

I'm not sure what to do to workaround this.
The page was not initially designed to have a level one heading.
It has to be normally used in the context of enclosed page which haswith 
a level one heading.


Not sure what you mean by an "enclosed page". Where can I see this in 
context?


Thanks,
David
-


Testing:
   Verified with the Axe plugin (add-on) for Mozilla Firefox browser.
   Asked Alex M. to double-check this, and his Axe reported no errors.

Thanks,
Serguei

Re: RFR JDK-8225474: JDI connector accept fails "Address already in use" with concurrent listeners

2019-07-01 Thread Andrew Leonard

Any one able to review please?
Thanks
Andrew

Andrew Leonard
Java Runtimes Development
IBM Hursley
IBM United Kingdom Ltd
internet email: andrew_m_leon...@uk.ibm.com 




From:   Andrew Leonard/UK/IBM
To: Alan Bateman 
Cc: serviceability-dev@openjdk.java.net
Date:   19/06/2019 18:29
Subject:Re: RFR JDK-8225474: JDI connector accept fails "Address 
already in use" with concurrent listeners


Alan and other reviewers please?,
Firstly apologies for the long post, but I wanted to be thorough.

Following previous comments I have reviewed my understanding of 
concurrency and "safe publishing" in the JMM, and I have a fairly clear 
understanding now. I have reworked my fix to the issue associated with 
concurrent JDI connector listeners after thoroughly reviewing and walking 
through the sun.tools.jdi Connector implementation and also reading the 
spec javadoc.
My Goal: To make JDI Connector and associated listening/attaching APIs 
"thread-safe", for the purpose of supporting a debugger use case where 
multiple listening/attaching "threads" are used.

>From the review I determined the following key components are already 
thread-safe:
- VirtualMachineImpl
- VirtualMachineManagerImpl

The components that needed some work were:
- ConnectorImpl and all its sub-classes
- A Minor fix to SocketTransportService

My new patch is available for review here: 
http://cr.openjdk.java.net/~aleonard/8225474/webrev.05/

Tests run successfully:
- new JdwpConcurrentAttachTest.java which performs a multi-threaded stress 
test of the start/stopListening process where the "bug" occurs
- jtreg -jdk:$PRODUCT_HOME -concurrency:12 -v1 -timeoutfactor:20 
langtools/jdk/jshell
- jtreg -jdk:$PRODUCT_HOME -timeoutfactor:20 -v1 jdk/com/sun/jdi  (2 tests 
failed that failed before for unrelated reasons)

The following summarizes the key changes and why:

ConnectorImpl:
final Map defaultArguments = new LinkedHashMap<>();
==> Made "final" so that Connector object defaultArguments is 
safely published to other threads after construction

defaultArguments():
+   synchronized(defaultArguments) {
Collection values = defaultArguments.values();
==> Synchronize on defaultArguments before iterating over values to 
return a "copy" to debugger

addString/Boolean/Integer/SelectedArgument:
+synchronized(defaultArguments) {
 defaultArguments.put(name,
==> synchronize on defaultArguments for updates

toString:
+synchronized(defaultArguments) {
Iterator iter = 
defaultArguments().values().iterator();
==> synchronize on defaultArguments prior to values iteration, and to 
create a "happens-before" relationship

+   synchronized(this) {
if (messages == null) {
messages = 
ResourceBundle.getBundle("com.sun.tools.jdi.resources.jdi");
}
+   }
==> Protect messages construction

ArgumentImpl
private final String name;
private final String label;
private final String description;
private volatile String value;
private final boolean mustSpecify;
==> final all except value which is volatile, so that Argument can 
be safely published to other threads after construction
==> value is "volatile" as this can be set by debuggers, so must 
set volatile to ensure if passed to other thread a "happens-before" is 
ensured

BooleanArgumentImpl
+synchronized(ConnectorImpl.class) {
if(trueString == null) {
trueString = getString("true");
falseString = getString("false");
}
+}
==> synchronized on ConnectorImpl.class object to ensure lock for 
initializing the static fields


GenericListeningConnector: 
final Map, 
TransportService.ListenKey>  listenMap;
final TransportService transportService;
final Transport transport;
==> "final" these to ensure "safe publication" to other threads 
after Connector construction

 
private GenericListeningConnector(TransportService ts,
   boolean addAddressArgument,
+  Transport tsp)
 ==> Added Transport param to constructor so that sub-classes can 
pass in their desired Transport and then allow transport to be "final" for 
"safe publication". Previously transport was being constructed "twice" 
wastefully.
 
listenMap = new ConcurrentHashMap, TransportService.ListenKey>(10);
==> Make listenMap a ConcurrentHashMap so that it is "thread-safe" 
for access/updates

 
public synchronized String startListening/stopListening(..
==> Made start & stop listening "synchronized" so that management 
of listenMap entry and transportService state are locked

 
GenericAttachingConnector:
final TransportService transportService;
final Transport transport;
==> Made "final" so that Connector can be safely published

(13) RFR(XS): 8226596: Accessibility errors in jdwp-protocol.html

2019-07-01 Thread serguei.spit...@oracle.com


  
  
Please, review a doc issue fix for:
    https://bugs.openjdk.java.net/browse/JDK-8226596
  
  Webrev:
   
  http://cr.openjdk.java.net/~sspitsyn/webrevs/2019/8226596-jdwp-a11y.1/
  
  The updated JDWP protocol page:
   
  http://cr.openjdk.java.net/~sspitsyn/webrevs/2019/jdwp-protocol.html
  
  
  Summary:
    The change is to replace the 'navigation' attribute with the
   element. 

  With this fix, the Axe reports one issue only:
  
      "Page must contain a level one heading".
  
  
    
  I'm not sure what to do to workaround this.
  
    
  The page was not initially designed to have a level one heading.
  
    
  It has to be normally used in the context of enclosed page which
  haswith a level one heading.
  
  Testing:
    Verified with the Axe plugin (add-on) for Mozilla Firefox
  browser.
    Asked Alex M. to double-check this, and his Axe reported no
  errors.
  
  Thanks,
  Serguei

Re: (13) RFR (S): 8226603: accessibility issues in specs/jvmti.html

2019-07-01 Thread serguei.spit...@oracle.com


Thanks a lot, Alex!
Serguei


On 7/1/19 11:25, Alex Menkov wrote:

+1

--alex

On 06/28/2019 21:44, David Holmes wrote:

Hi Serguei,

Looks good and trivial.

Thanks,
David

On 28/06/2019 9:55 pm, serguei.spit...@oracle.com wrote:

Please, review a fix for:
   https://bugs.openjdk.java.net/browse/JDK-8226603

Webrev:
http://cr.openjdk.java.net/~sspitsyn/webrevs/2019/8226603-jvmti-a11y.1/


Summary:
   The change fixes the a11y issues reported by the Axe tool.


Testing:
   Alex checked that the Axe tool reports noting with the updated 
jvmti.html.
   Also, the DocCheck reported the following heading errors for the 
original jvmti.html:


% grep heading report0/accessibility.log
0/jvmti.html:30: headings omitted
0/jvmti.html:27: Note: This is the previous heading
0/jvmti.html:880: headings omitted
0/jvmti.html:879: Note: This is the previous heading
0/jvmti.html:5308: headings omitted
0/jvmti.html:5253: Note: This is the previous heading
0/jvmti.html:7700: headings omitted
0/jvmti.html:7607: Note: This is the previous heading
0/jvmti.html:19857: headings omitted
0/jvmti.html:19797: Note: This is the previous heading
0/jvmti.html:23821: headings omitted
0/jvmti.html:23593: Note: This is the previous heading
0/jvmti.html:24546: headings omitted
0/jvmti.html:24416: Note: This is the previous heading
0/jvmti.html:25404: headings omitted
0/jvmti.html:25362: Note: This is the previous heading
0/jvmti.html:30724: headings omitted
0/jvmti.html:30723: Note: This is the previous heading

Now all these errors are gone.

Thanks,
Serguei

Re: (13) RFR (S): 8226603: accessibility issues in specs/jvmti.html

2019-07-01 Thread Alex Menkov


+1

--alex

On 06/28/2019 21:44, David Holmes wrote:

Hi Serguei,

Looks good and trivial.

Thanks,
David

On 28/06/2019 9:55 pm, serguei.spit...@oracle.com wrote:

Please, review a fix for:
   https://bugs.openjdk.java.net/browse/JDK-8226603

Webrev:
http://cr.openjdk.java.net/~sspitsyn/webrevs/2019/8226603-jvmti-a11y.1/


Summary:
   The change fixes the a11y issues reported by the Axe tool.


Testing:
   Alex checked that the Axe tool reports noting with the updated 
jvmti.html.
   Also, the DocCheck reported the following heading errors for the 
original jvmti.html:


% grep heading report0/accessibility.log
0/jvmti.html:30: headings omitted
0/jvmti.html:27: Note: This is the previous heading
0/jvmti.html:880: headings omitted
0/jvmti.html:879: Note: This is the previous heading
0/jvmti.html:5308: headings omitted
0/jvmti.html:5253: Note: This is the previous heading
0/jvmti.html:7700: headings omitted
0/jvmti.html:7607: Note: This is the previous heading
0/jvmti.html:19857: headings omitted
0/jvmti.html:19797: Note: This is the previous heading
0/jvmti.html:23821: headings omitted
0/jvmti.html:23593: Note: This is the previous heading
0/jvmti.html:24546: headings omitted
0/jvmti.html:24416: Note: This is the previous heading
0/jvmti.html:25404: headings omitted
0/jvmti.html:25362: Note: This is the previous heading
0/jvmti.html:30724: headings omitted
0/jvmti.html:30723: Note: This is the previous heading

Now all these errors are gone.

Thanks,
Serguei

Re: RFR: 8225715: jhsdb jmap fails to write binary heap dump of a jshell process

2019-07-01 Thread Chris Plummer


Looks good.

Chris

On 6/30/19 9:31 PM, Fairoz Matte wrote:

Hi,

Please review a fix that potentially handle NPE and allow heap to dump from a
process were we get source file name as null.

Background:
For taking heap dump of JShell process, we get getSourceFileName() as null.
Current implementation doesn't have null check.
This fix will handle the null check and avoid breaking in call to 
writeObjectID(sym).
Regression test provided with patch will demonstrate the problem.

JBS bug - https://bugs.openjdk.java.net/browse/JDK-8225715
Webrev - http://cr.openjdk.java.net/~fmatte/8225715/webrev.00/

Testing: Mach5 tier1,tier2 tier3, hs-tier1, hs-tier2 and hs-tier3 tests 
successfully passed

Thanks,
Fairoz

RE: RFR [XS] : 8226943: compile error in libfollowref003.cpp with XCode 10.2 on macosx

2019-07-01 Thread Baesken, Matthias

Hello,  thanks for the review !

> I'd suggest to fix it in 13 as it is the test fix.

I'll push it then to 13 , fine with me !

Best regards, Matthias


> 
> Hi Matthias,
> 
> The fix is good.
> It worked before because both JVMTI_REFERENCE_ARRAY_ELEMENT
> and JVMTI_HEAP_REFERENCE_ARRAY_ELEMENT have the same value 3
> as Gary noticed.
> 
> I'd suggest to fix it in 13 as it is the test fix.
> I've added labels 'testbug' and 'noreg-self'.
> 
> Thanks,
> Serguei
> 
> On 6/28/19 12:04 PM, David Holmes wrote:
> > Hi Matthias,
> >
> > Dropped build-dev and added serviceability-dev as this is a
> > servicability test.
> >
> > On 28/06/2019 7:43 am, Baesken, Matthias wrote:
> >> Hello please review this  small fix for a compile issue  on OSX .
> >> Today I  compiled   jdk/jdk   on a machine  with   XCode 10.2  . It
> >> worked pretty well .
> >> However this small issue showed up .
> >>
> >>
> >> In file included from
> >>
> /open_jdk/jdk_just_clone/jdk/test/hotspot/jtreg/vmTestbase/nsk/jvmti/u
> nit/FollowReferences/followref003/libfollowref003.cpp:33:
> >>
> /open_jdk/jdk_just_clone/jdk/test/hotspot/jtreg/vmTestbase/nsk/jvmti/u
> nit/FollowReferences/followref003/followref003.cpp:813:14:
> >> error:
> >> comparison of two values with different enumeration types in switch
> >> statement ('jvmtiHeapReferenceKind' and 'jvmtiObjectReferenceKind')
> >> [-Werror,-Wenum-compare-switch]
> >>
> >>
> >> And here XCode 10 is correct , JVMTI_REFERENCE_ARRAY_ELEMENT   is
> >> from a different  enumeration type  and should be replaced  with the
> >> value  from the correct enumeration type   .
> >>
> >> Bug / webrev :
> >>
> >> https://bugs.openjdk.java.net/browse/JDK-8226943
> >>
> >> http://cr.openjdk.java.net/~mbaesken/webrevs/8226943.0/
> >
> > The fix seems reasonable but the issue indicates a further problem
> > with the test. If it expected JVMTI_HEAP_REFERENCE_ARRAY_ELEMENT
> but
> > was checking for JVMTI_REFERENCE_ARRAY_ELEMENT then we should
> have hit
> > the default clause and failed the test. That suggests the test doesn't
> > actually expect JVMTI_HEAP_REFERENCE_ARRAY_ELEMENT in the first
> place.
> >
> > Cheers,
> > David
> >
> >>
> >> Thanks, Matthias
> >>

Re: (13) RFR(XS): 8226596: Accessibility errors in jdwp-protocol.html

Re: (13) RFR(XS): 8226596: Accessibility errors in jdwp-protocol.html

Re: (13) RFR(XS): 8226596: Accessibility errors in jdwp-protocol.html

Re: (13) RFR(XS): 8226596: Accessibility errors in jdwp-protocol.html

Re: (13) RFR(XS): 8226596: Accessibility errors in jdwp-protocol.html

PING: RFR: 8209790: SA tools not providing option to connect to debug server

Re: (13) RFR(XS): 8226596: Accessibility errors in jdwp-protocol.html

Re: RFR JDK-8225474: JDI connector accept fails "Address already in use" with concurrent listeners

(13) RFR(XS): 8226596: Accessibility errors in jdwp-protocol.html

Re: (13) RFR (S): 8226603: accessibility issues in specs/jvmti.html

Re: (13) RFR (S): 8226603: accessibility issues in specs/jvmti.html

Re: RFR: 8225715: jhsdb jmap fails to write binary heap dump of a jshell process

RE: RFR [XS] : 8226943: compile error in libfollowref003.cpp with XCode 10.2 on macosx

13 matches

Site Navigation

Mail list logo

Footer information