Re: RFR: 8264774: Implementation of Foreign Function and Memory API (Incubator) [v2]
On Wed, 28 Apr 2021 18:44:18 GMT, Daniel Fuchs wrote: >> I think the method is called during module bootstrap - I don't think there >> is a race in practice. This method is also called in other parts of >> ModuleBootstrap. The code you allude to is called during initialization of >> the IllegalNativeAccessChecker singleton, which should happen only once, and >> only from one thread. > > I'll take your word for it - the use of a volatile variable to store the > singleton instance made this suspicious. good point - I think that came in when I adapted the code from IllegalAccessLogger - which can indeed be accessed from multiple threads. In this case the initialization is effectively part of bootstrap, and volatile is not required. - PR: https://git.openjdk.java.net/jdk/pull/3699
Re: RFR: 8264774: Implementation of Foreign Function and Memory API (Incubator) [v2]
On Wed, 28 Apr 2021 19:02:57 GMT, Daniel Fuchs wrote: >> Maurizio Cimadamore has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Address first batch of review comments > > src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/MemoryAddress.java > line 209: > >> 207: /** >> 208: * The native memory address instance modelling the {@code NULL} >> address, associated >> 209: * with the {@link ResourceScope#globalScope() global} resource >> scope. > > `{@linkplain }` ? thanks - I'll do a pass - I think I probably got all of them wrong - PR: https://git.openjdk.java.net/jdk/pull/3699
Re: RFR: 8264774: Implementation of Foreign Function and Memory API (Incubator) [v2]
On Wed, 28 Apr 2021 10:42:54 GMT, Maurizio Cimadamore wrote: >> This PR contains the API and implementation changes for JEP-412 [1]. A more >> detailed description of such changes, to avoid repetitions during the review >> process, is included as a separate comment. >> >> [1] - https://openjdk.java.net/jeps/412 > > Maurizio Cimadamore has updated the pull request incrementally with one > additional commit since the last revision: > > Address first batch of review comments src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/LibraryLookup.java line 52: > 50: * > 51: * For {@link #lookup(String) memory addresses} obtained from a library > lookup object, > 52: * since {@link CLinker#downcallHandle(Addressable, MethodType, > FunctionDescriptor) native method handles} These should be `{@linkplain }` since the text of the link is plain text (and not code) src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/LibraryLookup.java line 88: > 86: * @return the memory segment associated with the library symbol (if > any). > 87: * @throws IllegalArgumentException if the address associated with > the lookup symbol do not match the > 88: * {@link MemoryLayout#byteAlignment() alignment constraints} in > {@code layout}. Same remark here (`{@linkplain }`) src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/MemoryAddress.java line 43: > 41: * when performing memory dereference operations using a memory access > var handle (see {@link MemoryHandles}). > 42: * > 43: * A memory address is associated with a {@link ResourceScope resource > scope}; the resource scope determines the `{@linkplain }` src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/MemoryAddress.java line 46: > 44: * lifecycle of the memory address, and whether the address can be used > from multiple threads. Memory addresses > 45: * obtained from {@link #ofLong(long) numeric values}, or from native > code, are associated with the > 46: * {@link ResourceScope#globalScope() global resource scope}. Memory > addresses obtained from segments ... and here to (`{@linkplain }`) src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/MemoryAddress.java line 102: > 100: * @param segment the segment relative to which this address offset > should be computed > 101: * @throws IllegalArgumentException if {@code segment} is not > compatible with this address; this can happen, for instance, > 102: * when {@code segment} models an heap memory region, while this > address is a {@link #isNative() native} address. `{@linkplain }` src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/MemoryAddress.java line 209: > 207: /** > 208: * The native memory address instance modelling the {@code NULL} > address, associated > 209: * with the {@link ResourceScope#globalScope() global} resource > scope. `{@linkplain }` ? - PR: https://git.openjdk.java.net/jdk/pull/3699
Re: RFR: 8264774: Implementation of Foreign Function and Memory API (Incubator) [v2]
On Wed, 28 Apr 2021 18:19:14 GMT, Maurizio Cimadamore wrote: >> src/java.base/share/classes/jdk/internal/module/IllegalNativeAccessChecker.java >> line 78: >> >>> 76: int index = 0; >>> 77: // the system property is removed after decoding >>> 78: String value = getAndRemoveProperty(prefix + index); >> >> I am not sure what is going on with the removal of the properties, but if >> I'm not mistaken this is racy: from the implementation of the checker() >> method above, it looks as if two different threads could trigger a call to >> the decode() function concurrently, which can result in a random >> partitioning of the properties against the two checkers being instantiated, >> with one of them being eventually set as the system-wide checker. > > I think the method is called during module bootstrap - I don't think there is > a race in practice. This method is also called in other parts of > ModuleBootstrap. The code you allude to is called during initialization of > the IllegalNativeAccessChecker singleton, which should happen only once, and > only from one thread. I'll take your word for it - the use of a volatile variable to store the singleton instance made this suspicious. - PR: https://git.openjdk.java.net/jdk/pull/3699
Re: RFR: 8264774: Implementation of Foreign Function and Memory API (Incubator) [v2]
On Wed, 28 Apr 2021 18:26:30 GMT, Maurizio Cimadamore wrote: > test/jdk/jdk/internal/reflect/CallerSensitive/CheckCSMs.java Yes that's the test. That test misses to catch your case where aliasing may be possible. - PR: https://git.openjdk.java.net/jdk/pull/3699
Re: RFR: 8264774: Implementation of Foreign Function and Memory API (Incubator) [v2]
On Wed, 28 Apr 2021 17:53:44 GMT, Daniel Fuchs wrote: >> Maurizio Cimadamore has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Address first batch of review comments > > src/java.base/share/classes/jdk/internal/module/IllegalNativeAccessChecker.java > line 40: > >> 38: >> 39: private IllegalNativeAccessChecker(Set allowedModuleNames, >> boolean allowAllUnnamedModules) { >> 40: this.allowedModuleNames = >> Collections.unmodifiableSet(allowedModuleNames); > > Should that be Set.copyOf() to take advantage of the immutability of `SetN` > (but at the expense of additional copying)... Honestly, I'm not even sure why we should be concerned about mutation of the set here - since we create this with a bunch of values read from a system property... e.g. should we just store `allowedModuleNames` period? - PR: https://git.openjdk.java.net/jdk/pull/3699
Re: RFR: 8264774: Implementation of Foreign Function and Memory API (Incubator) [v2]
On Wed, 28 Apr 2021 18:23:41 GMT, Mandy Chung wrote: > > I believe that we had to move @CallerSensitive out of interfaces because > > there was a test that was checking that @cs was not put on "virtual" > > methods. > > Good if we do have such test. We need to re-examine this with the security > team. I suggest to create a JBS issue and follow up separately. The test in question is: test/jdk/jdk/internal/reflect/CallerSensitive/CheckCSMs.java - PR: https://git.openjdk.java.net/jdk/pull/3699
Re: RFR: 8264774: Implementation of Foreign Function and Memory API (Incubator) [v2]
On Wed, 28 Apr 2021 10:42:54 GMT, Maurizio Cimadamore wrote: >> This PR contains the API and implementation changes for JEP-412 [1]. A more >> detailed description of such changes, to avoid repetitions during the review >> process, is included as a separate comment. >> >> [1] - https://openjdk.java.net/jeps/412 > > Maurizio Cimadamore has updated the pull request incrementally with one > additional commit since the last revision: > > Address first batch of review comments > I believe that we had to move @CallerSensitive out of interfaces because > there was a test that was checking that @cs was not put on "virtual" methods. Good if we do have such test.We need to re-examine this with the security team. I suggest to create a JBS issue and follow up separately. - PR: https://git.openjdk.java.net/jdk/pull/3699
Re: RFR: 8264774: Implementation of Foreign Function and Memory API (Incubator) [v2]
On Wed, 28 Apr 2021 18:07:32 GMT, Daniel Fuchs wrote: >> Maurizio Cimadamore has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Address first batch of review comments > > src/java.base/share/classes/jdk/internal/module/IllegalNativeAccessChecker.java > line 78: > >> 76: int index = 0; >> 77: // the system property is removed after decoding >> 78: String value = getAndRemoveProperty(prefix + index); > > I am not sure what is going on with the removal of the properties, but if I'm > not mistaken this is racy: from the implementation of the checker() method > above, it looks as if two different threads could trigger a call to the > decode() function concurrently, which can result in a random partitioning of > the properties against the two checkers being instantiated, with one of them > being eventually set as the system-wide checker. I think the method is called during module bootstrap - I don't think there is a race in practice. This method is also called in other parts of ModuleBootstrap. The code you allude to is called during initialization of the IllegalNativeAccessChecker singleton, which should happen only once, and only from one thread. - PR: https://git.openjdk.java.net/jdk/pull/3699
Re: RFR: 8264774: Implementation of Foreign Function and Memory API (Incubator) [v2]
On Wed, 28 Apr 2021 18:02:03 GMT, Mandy Chung wrote: > I reviewed the `--enable-native-access` related change that looks fine. > > > Access to restricted methods from any other module not in the list is > > disallowed and will result in an IllegalAccessException. > > I think you meant to say `IllegalCallerException` instead of > `IllegalAccessException`. Also do you intend to have javadoc to generate > `@throw IllegalCallerException` for the restricted methods automatically > besides the javadoc description? > IllegalCalller is probably better yes - we started off with an access-like check, so things have evolved a bit. I'll also add the @throws. > Making the restricted methods as `@CallerSensitive` in order to get the > caller class for native access check is the proper approach. However, some > interface methods are restricted methods such as `CLinker::downcallHandle` > whose the implementation method is `@CallerSensitive`. I concern with the > security issue with method handle and type aliasing. On the other hand, > `CLinker` is a sealed interface and only implemented by the platform and so > it's less of a concern. I think the interface method should also be > `@CallerSensitive` so that for example a method handle for > `CLinker::downcallHandle` will be produced with the proper caller-sensitive > context. I believe that we had to move @CallerSensitive out of interfaces because there was a test that was checking that @CS was not put on "virtual" methods. - PR: https://git.openjdk.java.net/jdk/pull/3699
Re: RFR: 8264774: Implementation of Foreign Function and Memory API (Incubator) [v2]
On Wed, 28 Apr 2021 10:42:54 GMT, Maurizio Cimadamore wrote: >> This PR contains the API and implementation changes for JEP-412 [1]. A more >> detailed description of such changes, to avoid repetitions during the review >> process, is included as a separate comment. >> >> [1] - https://openjdk.java.net/jeps/412 > > Maurizio Cimadamore has updated the pull request incrementally with one > additional commit since the last revision: > > Address first batch of review comments src/java.base/share/classes/jdk/internal/module/IllegalNativeAccessChecker.java line 40: > 38: > 39: private IllegalNativeAccessChecker(Set allowedModuleNames, > boolean allowAllUnnamedModules) { > 40: this.allowedModuleNames = > Collections.unmodifiableSet(allowedModuleNames); Should that be Set.copyOf() to take advantage of the immutability of `SetN` (but at the expense of additional copying)... src/java.base/share/classes/jdk/internal/module/IllegalNativeAccessChecker.java line 78: > 76: int index = 0; > 77: // the system property is removed after decoding > 78: String value = getAndRemoveProperty(prefix + index); I am not sure what is going on with the removal of the properties, but if I'm not mistaken this is racy: from the implementation of the checker() method above, it looks as if two different threads could trigger a call to the decode() function concurrently, which can result in a random partitioning of the properties against the two checkers being instantiated, with one of them being eventually set as the system-wide checker. - PR: https://git.openjdk.java.net/jdk/pull/3699
Re: RFR: 8264774: Implementation of Foreign Function and Memory API (Incubator) [v2]
On Wed, 28 Apr 2021 10:42:54 GMT, Maurizio Cimadamore wrote: >> This PR contains the API and implementation changes for JEP-412 [1]. A more >> detailed description of such changes, to avoid repetitions during the review >> process, is included as a separate comment. >> >> [1] - https://openjdk.java.net/jeps/412 > > Maurizio Cimadamore has updated the pull request incrementally with one > additional commit since the last revision: > > Address first batch of review comments I reviewed the `--enable-native-access` related change that looks fine. > Access to restricted methods from any other module not in the list is > disallowed and will result in an IllegalAccessException. I think you meant to say `IllegalCallerException` instead of `IllegalAccessException`. Also do you intend to have javadoc to generate `@throw IllegalCallerException` for the restricted methods automatically besides the javadoc description? Making the restricted methods as `@CallerSensitive` in order to get the caller class for native access check is the proper approach. However, some interface methods are restricted methods such as `CLinker::downcallHandle` whose the implementation method is `@CallerSensitive`.I concern with the security issue with method handle and type aliasing. On the other hand, `CLinker` is a sealed interface and only implemented by the platform and so it's less of a concern. I think the interface method should also be `@CallerSensitive` so that for example a method handle for `CLinker::downcallHandle` will be produced with the proper caller-sensitive context. - PR: https://git.openjdk.java.net/jdk/pull/3699
Re: RFR: 8264774: Implementation of Foreign Function and Memory API (Incubator) [v2]
On Wed, 28 Apr 2021 10:42:54 GMT, Maurizio Cimadamore wrote: >> This PR contains the API and implementation changes for JEP-412 [1]. A more >> detailed description of such changes, to avoid repetitions during the review >> process, is included as a separate comment. >> >> [1] - https://openjdk.java.net/jeps/412 > > Maurizio Cimadamore has updated the pull request incrementally with one > additional commit since the last revision: > > Address first batch of review comments src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/package-info.java line 224: > 222: * Some methods in this package are considered restricted. > Restricted methods are typically used to bind native > 223: * foreign data and/or functions to first-class Java API elements which > can then be used directly by client. For instance > 224: * the restricted method {@link > jdk.incubator.foreign.MemoryAddress#asSegment(long, ResourceScope)} can be > used to create typo: `used directly by client.` => `used directly by clients.` ? - PR: https://git.openjdk.java.net/jdk/pull/3699
Re: RFR: 8264774: Implementation of Foreign Function and Memory API (Incubator) [v2]
On Wed, 28 Apr 2021 13:47:43 GMT, Maurizio Cimadamore wrote: >> src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/CLinker.java >> line 270: >> >>> 268: >>> 269: /** >>> 270: * Converts a Java string into a null-terminated C string, using >>> the platform's default charset, >> >> Sorry if this has come up before, but, is the platform's default charset the >> right choice here? For other areas, we choose UTF-8 as the default. In fact, >> there is a draft JEP to move the default charset to UTF-8. So if there is an >> implicit need to match the underlying platform's charset then this may need >> to be revisited. For now, I just want to check that this is not an >> accidental reliance on the platform's default charset, but a deliberate one. > > I believe here the goal is to be consistent with `String::getBytes`: > > > /** > * Encodes this {@code String} into a sequence of bytes using the > * platform's default charset, storing the result into a new byte array. > * > * The behavior of this method when this string cannot be encoded in > * the default charset is unspecified. The {@link > * java.nio.charset.CharsetEncoder} class should be used when more control > * over the encoding process is required. > * > * @return The resultant byte array > * > * @since 1.1 > */ > public byte[] getBytes() { > return encode(Charset.defaultCharset(), coder(), value); > } > > > So, you are right in that there's an element of platform-dependency here - > but I think it's a dependency that users learned to "ignore" mostly. If > developers want to be precise, and platform independent, they can always use > the Charset-accepting method. Of course this could be revisited, but I think > there is some consistency in what the API is trying to do. If, in the future, > defaultCharset will just be Utf8 - well, that's ok too - as long as the > method is specified to be "defaultCharset" dependent, what's behind > "defaultCharset" is an implementation detail that the user will have to be > aware of one way or another. Naoto is working on a couple of changes in advance of JEP 400. One of these is to expose a system property with the host charset and I suspect that the CLinker method will want to use that instead of Charset.defaultCharset. - PR: https://git.openjdk.java.net/jdk/pull/3699
Re: RFR: 8264774: Implementation of Foreign Function and Memory API (Incubator) [v2]
On Wed, 28 Apr 2021 12:47:36 GMT, Chris Hegarty wrote: >> Maurizio Cimadamore has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Address first batch of review comments > > src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/CLinker.java > line 270: > >> 268: >> 269: /** >> 270: * Converts a Java string into a null-terminated C string, using >> the platform's default charset, > > Sorry if this has come up before, but, is the platform's default charset the > right choice here? For other areas, we choose UTF-8 as the default. In fact, > there is a draft JEP to move the default charset to UTF-8. So if there is an > implicit need to match the underlying platform's charset then this may need > to be revisited. For now, I just want to check that this is not an > accidental reliance on the platform's default charset, but a deliberate one. I believe here the goal is to be consistent with `String::getBytes`: /** * Encodes this {@code String} into a sequence of bytes using the * platform's default charset, storing the result into a new byte array. * * The behavior of this method when this string cannot be encoded in * the default charset is unspecified. The {@link * java.nio.charset.CharsetEncoder} class should be used when more control * over the encoding process is required. * * @return The resultant byte array * * @since 1.1 */ public byte[] getBytes() { return encode(Charset.defaultCharset(), coder(), value); } So, you are right in that there's an element of platform-dependency here - but I think it's a dependency that users learned to "ignore" mostly. If developers want to be precise, and platform independent, they can always use the Charset-accepting method. Of course this could be revisited, but I think there is some consistency in what the API is trying to do. If, in the future, defaultCharset will just be Utf8 - well, that's ok too - as long as the method is specified to be "defaultCharset" dependent, what's behind "defaultCharset" is an implementation detail that the user will have to be aware of one way or another. - PR: https://git.openjdk.java.net/jdk/pull/3699
Re: RFR: 8264774: Implementation of Foreign Function and Memory API (Incubator) [v2]
On Wed, 28 Apr 2021 13:08:26 GMT, Chris Hegarty wrote: >> Maurizio Cimadamore has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Address first batch of review comments > > src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/MemorySegment.java > line 693: > >> 691: */ >> 692: static MemorySegment allocateNative(MemoryLayout layout, >> ResourceScope scope) { >> 693: Objects.requireNonNull(scope); > > Should the allocateNative methods declare that they throw ISE, if the given > ResourceScope is not alive? ( I found myself asking this q, then > considering the behaviour of a SegmentAllocator that is asked to allocate > after a RS has been closed ) Good point, yes it should - PR: https://git.openjdk.java.net/jdk/pull/3699
Re: RFR: 8264774: Implementation of Foreign Function and Memory API (Incubator) [v2]
On Wed, 28 Apr 2021 10:42:54 GMT, Maurizio Cimadamore wrote: >> This PR contains the API and implementation changes for JEP-412 [1]. A more >> detailed description of such changes, to avoid repetitions during the review >> process, is included as a separate comment. >> >> [1] - https://openjdk.java.net/jeps/412 > > Maurizio Cimadamore has updated the pull request incrementally with one > additional commit since the last revision: > > Address first batch of review comments Like Paul, I tracked the changes to this API in the Panama repo. Most of my remaining comments are small and come from re-reading the API docs. src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/CLinker.java line 270: > 268: > 269: /** > 270: * Converts a Java string into a null-terminated C string, using the > platform's default charset, Sorry if this has come up before, but, is the platform's default charset the right choice here? For other areas, we choose UTF-8 as the default. In fact, there is a draft JEP to move the default charset to UTF-8. So if there is an implicit need to match the underlying platform's charset then this may need to be revisited. For now, I just want to check that this is not an accidental reliance on the platform's default charset, but a deliberate one. src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/MemorySegment.java line 101: > 99: * Lifecycle and confinement > 100: * > 101: * Memory segments are associated to a resource scope (see {@link > ResourceScope}), which can be accessed using Typo?? "Memory segments are associated *with* a resource scope" src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/MemorySegment.java line 112: > 110: MemoryAccess.getLong(segment); // already closed! > 111: * } > 112: * Additionally, access to a memory segment in subject to the > thread-confinement checks enforced by the owning scope; that is, Typo?? "Additionally, access to a memory segment *is* subject to ..." src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/MemorySegment.java line 114: > 112: * Additionally, access to a memory segment in subject to the > thread-confinement checks enforced by the owning scope; that is, > 113: * if the segment is associated with a shared scope, it can be accessed > by multiple threads; if it is associated with a confined > 114: * scope, it can only be accessed by the thread which own the scope. Typo?? "it can only be accessed by the thread which ownS the scope." src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/MemorySegment.java line 693: > 691: */ > 692: static MemorySegment allocateNative(MemoryLayout layout, > ResourceScope scope) { > 693: Objects.requireNonNull(scope); Should the allocateNative methods declare that they throw ISE, if the given ResourceScope is not alive? ( I found myself asking this q, then considering the behaviour of a SegmentAllocator that is asked to allocate after a RS has been closed ) - Marked as reviewed by chegar (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/3699
Re: RFR: 8264774: Implementation of Foreign Function and Memory API (Incubator) [v2]
On Wed, 28 Apr 2021 09:10:37 GMT, Chris Hegarty wrote: >> Maurizio Cimadamore has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Address first batch of review comments > > src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/ResourceScope.java > line 135: > >> 133: } finally { >> 134:segment.scope().release(segmentHandle); >> 135: } > > I do like the symmetry in this example, but just to add an alternative idiom: > `segmentHandle.scope().release(segmentHandle)` > , which guarantees to avoid release throwing and IAE I see what you mean - I don't think I want to encourage that style too much by giving it prominence in the javadoc. It's true you can go back to the scope from the handle, so that you are guaranteed to release the right thing, but I think that should be unnecessary in most idiomatic uses. - PR: https://git.openjdk.java.net/jdk/pull/3699
Re: RFR: 8264774: Implementation of Foreign Function and Memory API (Incubator) [v2]
On Wed, 28 Apr 2021 09:06:29 GMT, Chris Hegarty wrote: >> Maurizio Cimadamore has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Address first batch of review comments > > src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/ResourceScope.java > line 205: > >> 203: * until all the resource scope handles acquired from it have been >> {@link #release(Handle)} released}. >> 204: * @return a resource scope handle. >> 205: */ > > Given recent changes, it might be good to generalise the opening sentence > here. Maybe : > " Acquires a resource scope handle associated with this resource scope. If > the resource scope is explicit ... " Or some such. The spec for the acquire method talks only of explicit resource scopes. The comment is suggesting that the spec could be generalised a little, since it is possible to acquire a resource scope handle associated with implicit scopes. - PR: https://git.openjdk.java.net/jdk/pull/3699
Re: RFR: 8264774: Implementation of Foreign Function and Memory API (Incubator) [v2]
> This PR contains the API and implementation changes for JEP-412 [1]. A more > detailed description of such changes, to avoid repetitions during the review > process, is included as a separate comment. > > [1] - https://openjdk.java.net/jeps/412 Maurizio Cimadamore has updated the pull request incrementally with one additional commit since the last revision: Address first batch of review comments - Changes: - all: https://git.openjdk.java.net/jdk/pull/3699/files - new: https://git.openjdk.java.net/jdk/pull/3699/files/7545f71f..a80d8180 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=3699=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=3699=00-01 Stats: 42 lines in 4 files changed: 25 ins; 11 del; 6 mod Patch: https://git.openjdk.java.net/jdk/pull/3699.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3699/head:pull/3699 PR: https://git.openjdk.java.net/jdk/pull/3699