Re: Regarding current JVMTI improvements

2016-01-21 Thread Michael Rasmussen
Hi Alan

>
> [1] https://bugs.openjdk.java.net/browse/JDK-8144730
> [2] https://bugs.openjdk.java.net/browse/JDK-8136930
> [3] https://bugs.openjdk.java.net/browse/JDK-8146454

Thank you for the references, I just wanted to mention them again; but of
course they are logged in the system, silly me just failed at finding them!

/Michael


hg: jigsaw/jake/jdk: 2 new changesets

2016-01-21 Thread alan . bateman
Changeset: c0ab34210ad9
Author:alanb
Date:  2016-01-21 13:45 +
URL:   http://hg.openjdk.java.net/jigsaw/jake/jdk/rev/c0ab34210ad9

Default class path to . when not set and no initial module

! src/java.base/share/classes/jdk/internal/misc/ClassLoaders.java

Changeset: f36cffb66775
Author:alanb
Date:  2016-01-21 13:55 +
URL:   http://hg.openjdk.java.net/jigsaw/jake/jdk/rev/f36cffb66775

Correct MH javadoc

! src/java.base/share/classes/java/lang/invoke/MethodHandles.java
! src/java.base/share/classes/sun/invoke/util/VerifyAccess.java



Re: Regarding current JVMTI improvements

2016-01-21 Thread Alan Bateman

On 21/01/2016 10:27, Michael Rasmussen wrote:

Hi

Now that some changes are going into the JVMTI implementation in order to
support modules, I just wanted to bring up the question I asked back in
early December regarding support for setting the -Xpatch from JVMTI in
the OnLoad phase, as well as the inability for JVMTI agents to instrument
classes loaded before Start phase, due to CFLH events not being fired.

There is an issue with changing jdk.boot.class.path.append in the 
Agent_OnLoad as we discussed a few weeks back. Lois is looking this as 
JDK-8144730 [1]. A separate question is whether the JDK-specific 
property jdk.boot.class.path.append will be documented property. We do 
mention it in JEP 261 but it is not currently documented in the 
System.getProperties() implementation note - this is something 
potentially to discuss in the context of JDK-8136930 [2].


As regards the JVM TI spec and the CFLH event then it is a deliberate 
change to only post this event in the Start and Live phases and to have 
the Start phase be after the module system is initialized. To do it 
otherwise would break existing agents that instrument code that executes 
before the Live phase. We know this disadvantages agents that want to 
instrument classes that are loaded and used early in the VM startup and 
we will have a solution for this case once we have the "module-aware" 
agent support, tracked as JDK-8146454 [3], in jake. I think we will have 
to tie this to either a new JVM TI capability or a new "early start" 
phase. I don't expect any issues for agents that are interested in 
compilation events but there will be restrictions on agents that do 
instrumentation before the current Start phase. From your other mails 
then I understand it that you are patching classes in java.base so 
assuming you aren't patching those classes to invoke code outside of 
java.base then it should work with only minor updates to your agent.


-Alan

[1] https://bugs.openjdk.java.net/browse/JDK-8144730
[2] https://bugs.openjdk.java.net/browse/JDK-8136930
[3] https://bugs.openjdk.java.net/browse/JDK-8146454


hg: jigsaw/jake/nashorn: /-systemmodulepath/-system/

2016-01-21 Thread erik . joelsson
Changeset: f6a942282894
Author:erikj
Date:  2016-01-21 13:38 +0100
URL:   http://hg.openjdk.java.net/jigsaw/jake/nashorn/rev/f6a942282894

/-systemmodulepath/-system/

! make/BuildNashorn.gmk



hg: jigsaw/jake: /-systemmodulepath/-system/

2016-01-21 Thread erik . joelsson
Changeset: 8235e92cec13
Author:erikj
Date:  2016-01-21 13:38 +0100
URL:   http://hg.openjdk.java.net/jigsaw/jake/rev/8235e92cec13

/-systemmodulepath/-system/

! make/CompileJavaModules.gmk
! make/Javadoc.gmk
! make/common/SetupJavaCompilers.gmk



hg: jigsaw/jake/hotspot: Reenable test because it now passes because of app class loader change described in 8072481.

2016-01-21 Thread harold . seigel
Changeset: a9ae43ce3241
Author:hseigel
Date:  2016-01-21 11:16 -0500
URL:   http://hg.openjdk.java.net/jigsaw/jake/hotspot/rev/a9ae43ce3241

Reenable test because it now passes because of app class loader change 
described in 8072481.

! test/runtime/StackGuardPages/testme.sh



hg: jigsaw/jake/hotspot: 6 new changesets

2016-01-21 Thread chris . hegarty
Changeset: c5550502a1f1
Author:chegar
Date:  2016-01-13 14:34 +
URL:   http://hg.openjdk.java.net/jigsaw/jake/hotspot/rev/c5550502a1f1

8146736: Move sun.misc performance counters to jdk.internal.perf
Reviewed-by: alanb, mchung, rriggs

! src/share/vm/prims/nativeLookup.cpp
! src/share/vm/prims/perf.cpp

Changeset: d5239fc1b697
Author:lana
Date:  2016-01-14 12:03 -0800
URL:   http://hg.openjdk.java.net/jigsaw/jake/hotspot/rev/d5239fc1b697

Merge


Changeset: 2dce8e72f3ef
Author:chegar
Date:  2016-01-21 11:22 +
URL:   http://hg.openjdk.java.net/jigsaw/jake/hotspot/rev/2dce8e72f3ef

Merge


Changeset: 31e4d11eab11
Author:lana
Date:  2016-01-21 09:45 -0800
URL:   http://hg.openjdk.java.net/jigsaw/jake/hotspot/rev/31e4d11eab11

Added tag jdk-9+102 for changeset d5239fc1b697

! .hgtags

Changeset: 310896caab0c
Author:chegar
Date:  2016-01-21 20:56 +
URL:   http://hg.openjdk.java.net/jigsaw/jake/hotspot/rev/310896caab0c

Merge

! .hgtags

Changeset: 62d2a0ffb810
Author:chegar
Date:  2016-01-21 21:01 +
URL:   http://hg.openjdk.java.net/jigsaw/jake/hotspot/rev/62d2a0ffb810

Merge




hg: jigsaw/jake/corba: 2 new changesets

2016-01-21 Thread chris . hegarty
Changeset: 68b4d7d59f16
Author:lana
Date:  2016-01-21 09:45 -0800
URL:   http://hg.openjdk.java.net/jigsaw/jake/corba/rev/68b4d7d59f16

Added tag jdk-9+102 for changeset 9c4662334d93

! .hgtags

Changeset: 268666f27e51
Author:chegar
Date:  2016-01-21 20:56 +
URL:   http://hg.openjdk.java.net/jigsaw/jake/corba/rev/268666f27e51

Merge




hg: jigsaw/jake/jaxws: 2 new changesets

2016-01-21 Thread chris . hegarty
Changeset: 1ce986ce18e3
Author:lana
Date:  2016-01-21 09:46 -0800
URL:   http://hg.openjdk.java.net/jigsaw/jake/jaxws/rev/1ce986ce18e3

Added tag jdk-9+102 for changeset 0868b93587cc

! .hgtags

Changeset: 6f1a8b346869
Author:chegar
Date:  2016-01-21 20:56 +
URL:   http://hg.openjdk.java.net/jigsaw/jake/jaxws/rev/6f1a8b346869

Merge




hg: jigsaw/jake/nashorn: 12 new changesets

2016-01-21 Thread chris . hegarty
Changeset: bb63b699c060
Author:hannesw
Date:  2016-01-12 15:38 +0100
URL:   http://hg.openjdk.java.net/jigsaw/jake/nashorn/rev/bb63b699c060

8146888: Wrong license headers in test files
Reviewed-by: mhaupt, jlaskey

! test/script/nosecurity/treeapi/for.js
! test/script/nosecurity/treeapi/forin.js
! test/script/nosecurity/treeapi/functionCall.js
! test/script/nosecurity/treeapi/functionDeclaration.js
! test/script/nosecurity/treeapi/functionExpr.js
! test/script/nosecurity/treeapi/identifier.js
! test/script/nosecurity/treeapi/if.js
! test/script/nosecurity/treeapi/instanceof.js

Changeset: 8faab9cd4b95
Author:hannesw
Date:  2016-01-12 16:30 +0100
URL:   http://hg.openjdk.java.net/jigsaw/jake/nashorn/rev/8faab9cd4b95

8143896: java.lang.Long is implicitly converted to double
Reviewed-by: mhaupt, jlaskey

! 
src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/objects/NativeArray.java
! 
src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/objects/NativeDate.java
! 
src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/objects/NativeNumber.java
! 
src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/objects/NativeRegExpExecResult.java
! 
src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/objects/NativeString.java
! 
src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/JSType.java
! 
src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/arrays/ContinuousArrayData.java
! 
src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/arrays/IntArrayData.java
! 
src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/arrays/NumberArrayData.java
! 
src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/arrays/ObjectArrayData.java
! 
src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/linker/NashornGuards.java
! 
src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/linker/NashornPrimitiveLinker.java
! test/script/basic/JDK-8030200.js
! test/script/basic/JDK-8079145.js.EXPECTED
+ test/script/basic/JDK-8143896.js
! test/script/nosecurity/parserapi.js
! test/script/nosecurity/parserapi.js.EXPECTED

Changeset: 9ab6b645c428
Author:hannesw
Date:  2016-01-13 19:34 +0100
URL:   http://hg.openjdk.java.net/jigsaw/jake/nashorn/rev/9ab6b645c428

8147008: Nashorn primitive linker should handle ES6 symbols
Reviewed-by: attila, sundar

! 
src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/objects/Global.java
! 
src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/objects/NativeBoolean.java
! 
src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/objects/NativeSymbol.java
! 
src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/linker/NashornPrimitiveLinker.java
! test/script/basic/es6/symbols.js

Changeset: 0f21903deef8
Author:mhaupt
Date:  2016-01-14 10:55 +0100
URL:   http://hg.openjdk.java.net/jigsaw/jake/nashorn/rev/0f21903deef8

8036977: Make process singleton options to be context wide
Summary: The bug was fixed in an earlier change. This change contributes a test.
Reviewed-by: hannesw, sundar

+ test/script/nosecurity/context-dependent-logging.js

Changeset: da61004610e3
Author:sundar
Date:  2016-01-14 15:35 +0530
URL:   http://hg.openjdk.java.net/jigsaw/jake/nashorn/rev/da61004610e3

8147070: Dynalink GuardedInvocation must check the Class object passed
Reviewed-by: hannesw, mhaupt, attila

! src/jdk.dynalink/share/classes/jdk/dynalink/linker/GuardedInvocation.java

Changeset: 2247904a107c
Author:attila
Date:  2016-01-14 13:22 +0100
URL:   http://hg.openjdk.java.net/jigsaw/jake/nashorn/rev/2247904a107c

8144917: Prepare AbstractJavaLinker/BeanLinker codebase for missing member 
implementation
Reviewed-by: mhaupt, sundar

! src/jdk.dynalink/share/classes/jdk/dynalink/beans/AbstractJavaLinker.java
! src/jdk.dynalink/share/classes/jdk/dynalink/beans/BeanLinker.java
! src/jdk.dynalink/share/classes/jdk/dynalink/beans/StaticClassLinker.java

Changeset: 30c3bcdb762c
Author:attila
Date:  2016-01-14 13:24 +0100
URL:   http://hg.openjdk.java.net/jigsaw/jake/nashorn/rev/30c3bcdb762c

8144919: Implement missing member handler for BeansLinker
Reviewed-by: lagergren, mhaupt, sundar

! src/jdk.dynalink/share/classes/jdk/dynalink/beans/AbstractJavaLinker.java
! src/jdk.dynalink/share/classes/jdk/dynalink/beans/BeanLinker.java
! src/jdk.dynalink/share/classes/jdk/dynalink/beans/BeansLinker.java
+ 
src/jdk.dynalink/share/classes/jdk/dynalink/beans/LinkerServicesWithMissingMemberHandlerFactory.java
+ 
src/jdk.dynalink/share/classes/jdk/dynalink/beans/MissingMemberHandlerFactory.java
! 
src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/objects/NativeObject.java
! 
src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/Undefined.java
! 
src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/linker/Bootstrap.java
! 

hg: jigsaw/jake/langtools: 17 new changesets

2016-01-21 Thread chris . hegarty
Changeset: 88a874f33d6d
Author:alundblad
Date:  2016-01-08 17:14 +0100
URL:   http://hg.openjdk.java.net/jigsaw/jake/langtools/rev/88a874f33d6d

8144226: Sjavac's handling of include/exclude patterns is buggy, redundant and 
inconsistent
Summary: Rewrote sjavac include/exclude pattern handling.
Reviewed-by: jlahoda

! src/jdk.compiler/share/classes/com/sun/tools/sjavac/Source.java
! src/jdk.compiler/share/classes/com/sun/tools/sjavac/Util.java
! src/jdk.compiler/share/classes/com/sun/tools/sjavac/comp/SjavacImpl.java
! src/jdk.compiler/share/classes/com/sun/tools/sjavac/options/Option.java
! src/jdk.compiler/share/classes/com/sun/tools/sjavac/options/OptionHelper.java
! src/jdk.compiler/share/classes/com/sun/tools/sjavac/options/Options.java
! 
src/jdk.compiler/share/classes/com/sun/tools/sjavac/options/SourceLocation.java
! test/tools/sjavac/CompileExcludingDependency.java
! test/tools/sjavac/CompileWithAtFile.java
! test/tools/sjavac/CompileWithInvisibleSources.java
! test/tools/sjavac/CompileWithOverrideSources.java
- test/tools/sjavac/ExclPattern.java
+ test/tools/sjavac/HiddenFiles.java
+ test/tools/sjavac/IncludeExcludePatterns.java
! test/tools/sjavac/OptionDecoding.java
! test/tools/sjavac/Serialization.java
! test/tools/sjavac/util/OptionTestUtil.java

Changeset: f6740b308ee2
Author:dlsmith
Date:  2016-01-08 12:29 -0700
URL:   http://hg.openjdk.java.net/jigsaw/jake/langtools/rev/f6740b308ee2

8037789: Surprising more-specific results for lambda bodies with no return 
expressions
Reviewed-by: mcimadamore, vromero

! src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Resolve.java

Changeset: 9571d628ecf6
Author:vromero
Date:  2016-01-08 14:24 -0800
URL:   http://hg.openjdk.java.net/jigsaw/jake/langtools/rev/9571d628ecf6

8146719: javac test BootClassPathPrepend.java should be deleted
Reviewed-by: jjg

- test/tools/javac/file/BootClassPathPrepend.java

Changeset: 5ab68e3a1096
Author:vromero
Date:  2016-01-08 15:15 -0800
URL:   http://hg.openjdk.java.net/jigsaw/jake/langtools/rev/5ab68e3a1096

8146722: javac remove test T6430241.java as irrelevant in 9
Reviewed-by: jjg

- test/tools/javac/api/T6430241.java

Changeset: 3a6560c043d2
Author:dlsmith
Date:  2016-01-08 17:02 -0700
URL:   http://hg.openjdk.java.net/jigsaw/jake/langtools/rev/3a6560c043d2

8143852: Implement type variable renaming for functional interface most 
specific test
Reviewed-by: mcimadamore, vromero

! src/jdk.compiler/share/classes/com/sun/tools/javac/code/Types.java
! src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Resolve.java
+ test/tools/javac/lambda/MostSpecific15.java
+ test/tools/javac/lambda/MostSpecific16.java
+ test/tools/javac/lambda/MostSpecific16.out
+ test/tools/javac/lambda/MostSpecific17.java
+ test/tools/javac/lambda/MostSpecific18.java
+ test/tools/javac/lambda/MostSpecific19.java
+ test/tools/javac/lambda/MostSpecific19.out
+ test/tools/javac/lambda/MostSpecific20.java
+ test/tools/javac/lambda/MostSpecific21.java
+ test/tools/javac/lambda/MostSpecific21.out
+ test/tools/javac/lambda/MostSpecific22.java
+ test/tools/javac/lambda/MostSpecific23.java
+ test/tools/javac/lambda/MostSpecific23.out
+ test/tools/javac/lambda/MostSpecific24.java
+ test/tools/javac/lambda/MostSpecific24.out
+ test/tools/javac/lambda/MostSpecific25.java
+ test/tools/javac/lambda/MostSpecific25.out
+ test/tools/javac/lambda/MostSpecific26.java
+ test/tools/javac/lambda/MostSpecific26.out
+ test/tools/javac/lambda/MostSpecific27.java
+ test/tools/javac/lambda/MostSpecific28.java
+ test/tools/javac/lambda/MostSpecific28.out

Changeset: 9f3a70d14025
Author:jjg
Date:  2016-01-08 22:24 -0800
URL:   http://hg.openjdk.java.net/jigsaw/jake/langtools/rev/9f3a70d14025

8146727: test tools/sjavac/IncludeExcludePatterns.java fails on Windows
Reviewed-by: darcy

! test/tools/sjavac/IncludeExcludePatterns.java

Changeset: c3b040ed4122
Author:jlahoda
Date:  2016-01-11 11:21 +0100
URL:   http://hg.openjdk.java.net/jigsaw/jake/langtools/rev/c3b040ed4122

8056897: Improve error recovery for empty binary and hexadecimal literals.
Reviewed-by: mcimadamore

! src/jdk.compiler/share/classes/com/sun/tools/javac/parser/JavaTokenizer.java
! test/tools/javac/BadHexConstant.java
! test/tools/javac/BadHexConstant.out
! test/tools/javac/diags/examples/IdentifierExpected.java
+ test/tools/javac/lexer/JavaLexerTest.java
! test/tools/javac/literals/T6891079.java
! test/tools/javac/literals/T6891079.out

Changeset: a5066095d36e
Author:alundblad
Date:  2016-01-11 17:08 +0100
URL:   http://hg.openjdk.java.net/jigsaw/jake/langtools/rev/a5066095d36e

8145944: sjavac client could not connect to server
Summary: Wait for port file to get deleted before returning from 
PortFile::delete
Reviewed-by: jlahoda

! src/jdk.compiler/share/classes/com/sun/tools/sjavac/server/PortFile.java
! src/jdk.compiler/share/classes/com/sun/tools/sjavac/server/SjavacServer.java

Changeset: 

hg: jigsaw/jake/jaxp: 7 new changesets

2016-01-21 Thread chris . hegarty
Changeset: 1e29ca011af4
Author:joehw
Date:  2016-01-08 10:51 -0800
URL:   http://hg.openjdk.java.net/jigsaw/jake/jaxp/rev/1e29ca011af4

8144967: javax.xml.transform.Source and org.xml.sax.InputSource can be empty
Reviewed-by: darcy, rriggs

! src/java.xml/share/classes/javax/xml/transform/Source.java
! src/java.xml/share/classes/javax/xml/transform/dom/DOMSource.java
! src/java.xml/share/classes/javax/xml/transform/sax/SAXSource.java
! src/java.xml/share/classes/javax/xml/transform/stax/StAXSource.java
! src/java.xml/share/classes/javax/xml/transform/stream/StreamSource.java
! src/java.xml/share/classes/org/xml/sax/InputSource.java
+ test/javax/xml/jaxp/unittest/common/Sources.java

Changeset: b34427de0b08
Author:joehw
Date:  2016-01-12 15:29 -0800
URL:   http://hg.openjdk.java.net/jigsaw/jake/jaxp/rev/b34427de0b08

8144966: Catalog API: Null handling and reference to Reader
Reviewed-by: mchung, rriggs

! src/java.xml/share/classes/javax/xml/catalog/CatalogImpl.java
! src/java.xml/share/classes/javax/xml/catalog/CatalogManager.java
! src/java.xml/share/classes/javax/xml/catalog/CatalogUriResolver.java
! test/javax/xml/jaxp/functional/catalog/DeferFeatureTest.java
! test/javax/xml/jaxp/libs/catalog/CatalogTestUtils.java
! test/javax/xml/jaxp/libs/jaxp/library/JAXPTestUtilities.java
! test/javax/xml/jaxp/unittest/catalog/CatalogTest.java

Changeset: 56972d2827cb
Author:joehw
Date:  2016-01-13 10:12 -0800
URL:   http://hg.openjdk.java.net/jigsaw/jake/jaxp/rev/56972d2827cb

8146606: Catalog.matchSystem() appends an extra '/' to the matched result
Reviewed-by: lancea

! src/java.xml/share/classes/javax/xml/catalog/RewriteSystem.java
! src/java.xml/share/classes/javax/xml/catalog/RewriteUri.java
! test/javax/xml/jaxp/unittest/catalog/CatalogTest.java
+ test/javax/xml/jaxp/unittest/catalog/rewriteCatalog.xml

Changeset: 9dcf193c0b6c
Author:lana
Date:  2016-01-14 12:03 -0800
URL:   http://hg.openjdk.java.net/jigsaw/jake/jaxp/rev/9dcf193c0b6c

Merge


Changeset: 62db4eb04781
Author:chegar
Date:  2016-01-21 11:19 +
URL:   http://hg.openjdk.java.net/jigsaw/jake/jaxp/rev/62db4eb04781

Merge


Changeset: 09dcbee7948e
Author:lana
Date:  2016-01-21 09:46 -0800
URL:   http://hg.openjdk.java.net/jigsaw/jake/jaxp/rev/09dcbee7948e

Added tag jdk-9+102 for changeset 9dcf193c0b6c

! .hgtags

Changeset: 99afcfda3af0
Author:chegar
Date:  2016-01-21 20:56 +
URL:   http://hg.openjdk.java.net/jigsaw/jake/jaxp/rev/99afcfda3af0

Merge




Re: Round #2: RFR: 8049365 - Update JDI and JDWP for modules

2016-01-21 Thread serguei.spit...@oracle.com

On 1/21/16 04:51, Alan Bateman wrote:

On 20/01/2016 22:35, serguei.spit...@oracle.com wrote:

:

Jdk webrev:
http://cr.openjdk.java.net/~sspitsyn/webrevs/2016/jdk/8049365-Jigsaw-jdk.2/

Hotspot webrev:
http://cr.openjdk.java.net/~sspitsyn/webrevs/2016/hotspot/8049365-Jigsaw-hs.2/


jvmti.xml - the module catagory has since="9" but the JVM TI spec 
version rev's to 1.3.2. I started a thread a few weeks ago on whether 
JNI and JVM TI should move to major version 9 and just track the Java 
SE version going forward. I don't think we have closure on that 
discussion yet but for now then I suggest we move to JVMTI_VERSION_9 
on the assumption that this is the way forward.


Ok
I saw the discussion but have no strong opinion yet.
Changing to JVMTI_VERSION_9 will make it more consistent though.




The GetAllModules implementation is okay for now and I believe Lois 
will replace JvmtiModuleClosure once your changes are in and she gets 
time to replace it.


I hope so.




Now to the jdk repo ...

We seem to have an issue with the generated jvmti.h generating a GPL 
copyright header. This means that copying jvmti.h into the jdk repo 
will change it from GPL + "Classpath" exception to pure GPL. I'll 
create a separate bug for this, this seems to be technical debt that 
we have a copy of jvmti.h in the jdk repo but fixing that issue means 
we need a jvmti.h with the right copyright because that is what goes 
into the JDK include directory.


I could easily fix it now but it is Ok to file a separate bug.




jdi.ReferenceType - I didn't generate the javadoc with your patch but 
it looks like the "Not all target ..." note will end up in the @return 
tag and I assume should move up. There is also a stray  after the 
@throws.


Ok



jdi.VirtualMachine - this has the same issue with the "Not all target 
..." note. In the canGetModuleInfo() method then I assume the @see 
should be moved to the end of the class description.


Ok



Can you check allModules in VirtualMachineImpl.c as it doesn't look 
quite right when GetAllModules fails, is there a return missing as 
otherwise it will write the module count and attempt to deallocate NULL.


Will check.



@jdk.Exported has been removed in JDK 9 and that removal has got to 
jake. I assume your forest is a bit behind.


Nice catch.
Will pull an update.




jdi.ModuleReference
- as this code is new then it would be good to use {@code  ..} instead 
of .
- I don't think name, classLoader or canRead can throw UOE so this 
exception can be removed from the javadoc.
- the class description reads "A module that currently exists in the 
target VM". Looking at the other JDI classes then it might be more 
consistent to use "A module in the target VM".


Ok




tools.jdi.ModuleReferenceImpl
- if change the fields to volatile then we can avoid these methods 
needing to be synchronized.
- a minor nit is that it would be good to avoid overly long lines as 
it's a pain to have v. long lines when doing side-by-side diffs.


Ok



tools.jdi.VirtualMachineImpl
- can modulesByID be Map, I can't quite tell 
why the value needs to be the impl type.
- It looks like vmMajorVersion will return 0 when the target VM is JDK 
8 or older, is that right? I guess that is okay because we only care 
about >= 9 but we will need to look at this closely.


Will check.



As per previous mails then I think we'll need additional tests in this 
area. It would be good to create another issue in JIRA to track that work.


Agreed.
Will provide an update in the next review round.


Thanks,
Serguei


-Alan