Integrated: 8265152: jpackage cleanup fails on Windows with IOException deleting msi

[Yasumasa Suenaga]

On Sun, 18 Apr 2021 12:44:17 GMT, Yasumasa Suenaga  wrote:

> When creating an "exe" installer on Windows, `AbstractBundler.cleanup()` 
> calls `IOUtils.deleteRecursive()` to delete the tmp directory. It can 
> intermittently fail trying to delete the msi file.
> 
> [JDK-8263135](https://bugs.openjdk.java.net/browse/JDK-8263135) removed 
> `unique_ptr` from jpackage sources due to compiler warnings in VS 2019 
> (16.9.0), so we need to release the resource manually. However `DatabaseView` 
> didn't do that.

This pull request has now been integrated.

Changeset: 142edd3a
Author:Yasumasa Suenaga 
URL:   https://git.openjdk.java.net/jdk/commit/142edd3a
Stats: 6 lines in 1 file changed: 6 ins; 0 del; 0 mod

8265152: jpackage cleanup fails on Windows with IOException deleting msi

Reviewed-by: herrick, asemenyuk

-

PR: https://git.openjdk.java.net/jdk/pull/3560

Re: ReversibleCollection proposal

[forax]

- Mail original -
> De: "Stuart Marks" 
> À: "Remi Forax" 
> Cc: "core-libs-dev" 
> Envoyé: Lundi 19 Avril 2021 17:41:11
> Objet: Re: ReversibleCollection proposal

> On 4/17/21 9:49 AM, Remi Forax wrote:
>> I think the analysis is spot on but I don't think the proposed solution is 
>> the
>> right one.
>> 
>> Introducing a new interface (two in this case) has a really huge cost and in
>> this case, i've trouble to see why i will want to write a method that takes a
>> ReversibleCollection as parameter, ReversibleCollection as a type does not 
>> seem
>> to be useful. About the cost of introducing a new collection interface, it
>> means that we also have to update all emptyXXX, singleXXX, uncheckedXXX with 
>> a
>> new interface, and that only for the JDK. Every libraries that proxy
>> collections has also to be updated to take care of this new type, otherwise 
>> the
>> integration with an application that uses this new type and the library is 
>> not
>> seamless anymore.
> 
> Hi Remi,
> 
> Thanks for looking at this. There are several issues intertwined here, so 
> let's
> try
> to break it down.
> 
> There are a bunch of factory methods emptyX, singletonX, etc. that give
> instances of
> particular types. As Tagir noted, List is a ReversibleCollection, so you can
> just
> use emptyList or List.of or whatever if you need an instance of
> ReversibleCollection. Also, SortedSet/NavigableSet are ReversibleSets, so if 
> you
> need an empty ReversibleSet, you can use Collections.emptyNavigableSet. 
> There's
> no
> singletonNavigableSet, but you could do
> 
> Collections.unmodifiableNavigableSet(new TreeSet<>(List.of(x)))
> 
> which is a bit of a mouthful, but it's possible. (The fact that there's no
> Collections.singletonNavigableSet may mean this use case is so narrow that few
> people really needed it.)
> 
> As for the wrappers, synchronizedX and checkedX are mostly disused, so I don't
> see a
> need to provide those wrappers. Having unmodifiable RC and RS is probably
> useful, so
> I could see adding those. That's a certain amount of work, but not a 
> tremendous
> cost.

It's a shame that checkedX are not used more, at least in tests, because it's a 
simple way to know if a method will work with the new generics of Valhalla 
(it's the same semantics).

Anyway, the problem is not only for the JDK, but for all libraries like guava, 
eclipse collections, etc and also JVM languages that provides a collection 
interopt like Clojure or Scala. 

> 
>> All these methods can be introduced has default methods on the existing
>> collection interfaces, there is no need of a special interface (or two) for
>> that.
> 
> Clearly it's possible to get by without new interfaces. After all the
> collections
> framework has gotten by without them for 20+ years already. It's certainly
> possible
> to fill out the individual interfaces (List, Deque, Sorted/NavSet) and classes
> (LinkedHashSet, LinkedHashMap) with methods that are all similar to each 
> other,
> and
> that would be useful. Or as Peter Levart pointed out, we could push all of 
> them
> up
> to Collection and write the specs in a loose enough way to encompass even
> unordered
> collections.
> 
> Any of those alternatives (do nothing, add individual methods, add
> loosely-spec'd
> methods to Collection) are *possible* to do. However, I think you know how
> minimal
> we are with the JDK APIs, and we wouldn't have proposed new interfaces without
> good
> cause. Thus, I think introducing new *types* here is useful, for the following
> reasons.
> 
>  * There's a useful clump of semantics here, combined with sensible operations
>  that
> rely on those semantics. There are a lot of places in the spec where there is
> hedging of the form "if the collection has an ordering, then... otherwise the
> results are undefined". This is unnecessary in the context of a new type.
> Furthermore, the new operations fit well with the new types' semantics, with 
> no
> hedging necessary.

You can only say that a reversible collection has an ordering. But usually you 
want the opposite, all collections that have an ordering are a reversible 
collection. But while this can be true for the implementations inside the JDK 
that will never be true in Java because there all already collection 
implementations with an ordering which do not implement ReversibleCollection.

Sadly, this ship has sailed.
The spec will still have to say "if the collection has an ordering", otherwise 
the new semantics is not a backward compatible.

> 
>  * These semantics appear across a broad range of existing collection types 
> and
> implementations. It's quite valuable to have a type that unifies the common
> pieces
> of List, Deque, SortedSet, and LinkedHashSet into a single abstraction.

yes in term of documentation, but in Java, it also means that you can use that 
interface as a type.

For a List, a Deque or a Set, there are known algorithms that takes such type 
has parameter so it makes 

Re: RFR: 8199594: Add doc describing how (?x) ignores spaces in character classes [v2]

[Ian Graves]

> Clarifying note on comments mode to explicitly note that whitespace within 
> character classes is ignored.

Ian Graves has updated the pull request incrementally with one additional 
commit since the last revision:

  Adding differences to Perl 5 note

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/3577/files
  - new: https://git.openjdk.java.net/jdk/pull/3577/files/8ab051aa..f4507e3a

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=3577=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=3577=00-01

  Stats: 6 lines in 1 file changed: 6 ins; 0 del; 0 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3577.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3577/head:pull/3577

PR: https://git.openjdk.java.net/jdk/pull/3577

Re: RFR: 8200559: Java agents doing instrumentation need a means to define auxiliary classes [v2]

[Rafael Winterhalter]

On Fri, 16 Apr 2021 20:30:15 GMT, Rafael Winterhalter 
 wrote:

>> To allow agents the definition of auxiliary classes, an API is needed to 
>> allow this. Currently, this is often achieved by using `sun.misc.Unsafe` or 
>> `jdk.internal.misc.Unsafe` ever since the `defineClass` method was removed 
>> from `sun.misc.Unsafe`.
>
> Rafael Winterhalter has refreshed the contents of this pull request, and 
> previous commits have been removed. The incremental views will show 
> differences compared to the previous content of the PR. The pull request 
> contains one new commit since the last revision:
> 
>   8200559: Java agents doing instrumentation need a means to define auxiliary 
> classes

At the moment, it is required for root to switch to the user that owns the
JVM process as the domain socket is only accessible to that user to avoid
that users without access to the JVM can inject themselves into a JVM. I am
not sure if operations teams would be thrilled to have a monitoring agent
required to run as root, even in these times of Kubernetes.

I mainly have two comments:

1. The problem is the possibility of self-attach. I think this is the
problem to solve, a library getting agent privileges without being an
agent. I think this should be prevented while dynamic attach should
continue to be possible in today's format. It has proven to be so useful,
it would be a shame if the current tooling convenience would disappear from
the JVM. As it's my understanding, JNI is supposed to be restricted in the
future, in line with Panama. Without this restriction, JNI already allows
for random class definition, for example, which similarly to an agent
offers surpassing the majority of JVM restrictions. The second restriction
would be a control to restrict how a JVM process starts new processes. I
think both are reasonable restrictions for a library to face which require
explicit enabling. Especially with the security manager on it's way out,
certain capabilities should be rethought to begin with. If both are no
longer freely available, self-attachment is no longer possible anyways and
dynamic agents could retain their capabilities.

2. The question of introducing an Instrumentation::defineClass method is
fully independent of that first question. If a dynamic agent was to be
restricted, the method could reject classloader/package combinations for
dynamically loaded agents the same way that
Instrumentation::retransformClasses would need to. At the same time,
introducing the method would allow agents to move to an official API with a
Java 17 baseline which will be the next long-standing base line. I fully
understand it needs a thorough discussion but it is a less complicated
problem then (1) and could therefore be decided prior to having found a
satisfactory solution for it.

Am Mo., 19. Apr. 2021 um 16:07 Uhr schrieb Peter Levart <
***@***.***>:

> an application or library can use the attach mechanism (directly or via
> the attach API in a child VM) to load an agent and leak the Instrumentation
> object. This is the genie that somehow needs to be put back in its bottle.
> One approach that I mentioned here to create is a less powerful
> Instrumentation object for untrusted agents. Trusted agents would continue
> to the full-power
>
> I hear Rafael that dynamic attach is important to support monitoring and
> instrumenting large numbers of JVMs with no preparations (i.e. without
> issueing special command-line options to enable it). As I understand,
> current attach mechanism is designed to allow a process running under the
> same UID as the JVM or under root to attach to the JVM.
>
> What if this dynamic attach mechanism was modified so that only a process
> running under root could dynamically attach to the JVM? Old behavior would
> be enabled by special command line option, so by default, dynamic attach
> would be limited to tools running under root. Rafael mentions discovery,
> monitoring and instrumenting large number of JVMs running on hosts, so if
> one such tool has to attach to different JVMs running under different UIDs,
> it has to run as root now anyway.
>
> With such default "secure" dynamic attach and when the JVM is not running
> as root (which is a recommended security practice anyway), a library in
> such JVM could not attach back to the same JVM even through spawning
> sub-processes.
>
> How to achieve such "secure" dynamic attach? One way that comes to mind is
> a modified handshake. Currently, I think at least on Linux, the tool that
> wishes to attach to the JVM searches for a special UNIX socket (
> $PWD/.java_pid, /tmp/.java_pid) and if not found, creates a
> special attach file ($PWD/.attach_pid, /tmp/.attach_pid) to
> signal the JVM to create a listening UNIX socket under mentioned special
> path, then it connects to the socket. The UNIX socket file has UID:GID set
> to effective UID:GID of the JVM process and permissions to 0600, so only a
> tool running under same UID or root can connect to such 

Re: RFR: 8265036: JFR: Remove use of -XX:StartFlightRecording= and -XX:FlightRecorderOptions=

[Chris Plummer]

On Sun, 18 Apr 2021 15:17:35 GMT, Erik Gahlin  wrote:

> Hi,
> 
> Could I have a review of fix that removes the use of "=" together with 
> -XX:StartFlightRecording and -XX:FlightRecorderOptions. It's been possible to 
> use "-XX:StartFlightRecording:" and "-XX:FlightRecorderOption:" since JFR was 
> introduced into OpenJDK (JDK 11), so this is not a change of the 
> specification, just an update to make the use consistent in tests, comments, 
> documentation etc.
> 
> I also removed the use of -XX:+FlightRecorder, which is not needed, and has 
> been deprecated since JDK 13. 
> 
> Testing: jdk/jdk/jfr, tier 1-4.
> 
> Thanks
> Erik

Sorry, I accidentally stuck an "integrate" label in this PR (was meant for my 
own PR). I immediately deleted it. Hopefully no harm done.

-

PR: https://git.openjdk.java.net/jdk/pull/3561

Re: RFR: 8199594: Add doc describing how (?x) ignores spaces in character classes

[Ian Graves]

On Mon, 19 Apr 2021 20:43:26 GMT, Ian Graves  wrote:

> Clarifying note on comments mode to explicitly note that whitespace within 
> character classes is ignored.

Will put a /csr in to be safe.

-

PR: https://git.openjdk.java.net/jdk/pull/3577

Re: RFR: 8199594: Add doc describing how (?x) ignores spaces in character classes

[Brian Burkhalter]

On Mon, 19 Apr 2021 20:43:26 GMT, Ian Graves  wrote:

> Clarifying note on comments mode to explicitly note that whitespace within 
> character classes is ignored.

Looks all right. Probably a CSR is in order.

-

Marked as reviewed by bpb (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/3577

Re: ReversibleCollection proposal

[Remi Forax]

- Mail original -
> De: "Stuart Marks" 
> À: "core-libs-dev" 
> Envoyé: Vendredi 16 Avril 2021 19:40:55
> Objet: ReversibleCollection proposal

> This is a proposal to add a ReversibleCollection interface to the Collections
> Framework. I'm looking for comments on overall design before I work on 
> detailed
> specifications and tests. Please send such comments as replies on this email
> thread.
> 
> Here's a link to a draft PR that contains the code diffs. It's prototype
> quality,
> but it should be good enough to build and try out:
> 
> https://github.com/openjdk/jdk/pull/3533
> 
> And here's a link to a class diagram showing the proposed additions:
> 
> 
> https://cr.openjdk.java.net/~smarks/ReversibleCollection/ReversibleCollectionDiagram.pdf
> 
> Thanks,
> 
> s'marks

Thinking a little bit about your proposal,
introducing an interface right in the middle of a hierarchy is not a backward 
compatible change
(you have an issue when the compiler has to use the lowest upper bound).

By example
  void m(List> list) { ... }

  var list = List.of(new LinkedHashSet(), List.of("foo"));
  m(list);  // does not compile anymore

currently the type of list is List> but with your proposal, 
the type will be List>

Rémi

RFR: 8199594: Add doc describing how (?x) ignores spaces in character classes

[Ian Graves]

Clarifying note on comments mode to explicitly note that whitespace within 
character classes is ignored.

-

Commit messages:
 - Note about comments mode and character class whitespace

Changes: https://git.openjdk.java.net/jdk/pull/3577/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=3577=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8199594
  Stats: 4 lines in 1 file changed: 4 ins; 0 del; 0 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3577.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3577/head:pull/3577

PR: https://git.openjdk.java.net/jdk/pull/3577

Re: RFR: 8265036: JFR: Remove use of -XX:StartFlightRecording= and -XX:FlightRecorderOptions=

[Chris Plummer]

On Sun, 18 Apr 2021 15:17:35 GMT, Erik Gahlin  wrote:

> Hi,
> 
> Could I have a review of fix that removes the use of "=" together with 
> -XX:StartFlightRecording and -XX:FlightRecorderOptions. It's been possible to 
> use "-XX:StartFlightRecording:" and "-XX:FlightRecorderOption:" since JFR was 
> introduced into OpenJDK (JDK 11), so this is not a change of the 
> specification, just an update to make the use consistent in tests, comments, 
> documentation etc.
> 
> I also removed the use of -XX:+FlightRecorder, which is not needed, and has 
> been deprecated since JDK 13. 
> 
> Testing: jdk/jdk/jfr, tier 1-4.
> 
> Thanks
> Erik

The changes look good. Have you considered doing a test run where the use of 
`=` and `XX:+FlightRecorder` are disallowed just to make sure you caught them 
all?

-

Marked as reviewed by cjplummer (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/3561

Re: ObjectMethods seems generating wrong methods for array-type field

[Kengo TODA]

Hello Rémi, Raffaello,


Thanks for your reply. I got it.
I knew that it's better to make Record classes immutable, and
now learn one more new thing :)

As suggested by Rémi, I'll read the amber-spec-experts mailing list.
I want to grasp the reason to use invokedynamic in these methods.
This is my first motivation to learn about the ObjectMethods class.
Hope that the mailing list will help me to find it.


Thank you very much!

***
Kengo TODA
skypen...@gmail.com


On Wed, Apr 14, 2021 at 6:12 PM Remi Forax  wrote:

> - Mail original -
> > De: "Kengo TODA" 
> > À: "core-libs-dev" 
> > Envoyé: Mercredi 14 Avril 2021 11:03:14
> > Objet: Re: ObjectMethods seems generating wrong methods for array-type
> field
>
> Hello,
>
> > I found that the JLS 16 does not cover the array type record component:
> > https://docs.oracle.com/javase/specs/jls/se16/html/jls-8.html#jls-8.10.3
> >
> > So it could be not a core-lib issue but a JLS issue.
>
> Nope, the implementation of a record is covered in the javadoc of
> java.lang.Record
> for equals, see
> https://docs.oracle.com/en/java/javase/16/docs/api/java.base/java/lang/Record.html#equals(java.lang.Object)
>
> As you can see the spec clearly says that for any fields which is an
> object (arrays are objects), it should be semantically equivalent to
> calling java.util.Objects.equals() on those fields.
>
> So it's not a bug, it's how record.equals() works, as Raffaello said,
> records are dumb containers, this is how we have specified them.
>
> Records have been specified as part of the project Amber, you can take a
> look to the mailing list [1] for more if you want.
>
> >
> > Do I need to forward this thread to another mailing list?
> > If so, please let me know which is the preferred one.
> > I've checked https://mail.openjdk.java.net/mailman/listinfo but not so
> sure.
> >
> >
> > Thank you very much.
>
> regards,
> Rémi
>
> [1] https://mail.openjdk.java.net/mailman/listinfo/amber-spec-experts
>
> >
> > ***
> > Kengo TODA
> > skypen...@gmail.com
> >
> >
> > On Wed, Apr 14, 2021 at 9:04 AM Kengo TODA  wrote:
> >
> >> Hello there,
> >>
> >>
> >> I'm Kengo TODA, an engineer learning about the Record feature.
> >> Today I found a nonintentional behavior, and it seems that the bug
> >> database has no info about it.
> >> Let me ask here to confirm it's by-design or not. If it's a bug, I want
> to
> >> try to send a patch to OpenJDK.
> >>
> >> Here is the code reproducing the nonintentional behavior:
> >> https://gist.github.com/KengoTODA/4d7ef6a5226d347ad9385241fee6bc63
> >>
> >> In short, the ObjectMethods class in OpenJDK v16 generates code
> >> that invokes the fields' method, even when the field is an array.
> >>
> >> Please help me to understand this behavior, or
> >> make an entry in the bug database to propose a patch.
> >> Thanks in advance! :)
> >>
> >> ***
> >> Kengo TODA
> >> skypen...@gmail.com
>

Re: RFR: 8265237: String.join and StringJoiner can be improved further [v3]

[Roger Riggs]

On Sun, 18 Apr 2021 19:55:20 GMT, Peter Levart  wrote:

>> While JDK-8148937 improved StringJoiner class by replacing internal use of 
>> getChars that copies out characters from String elements into a char[] array 
>> with StringBuilder which is somehow more optimal, the improvement was 
>> marginal in speed (0% ... 10%) and mainly for smaller strings, while GC was 
>> reduced by about 50% in average per operation.
>> Initial attempt to tackle that issue was more involved, but was later 
>> discarded because it was apparently using too much internal String details 
>> in code that lives outside String and outside java.lang package.
>> But there is another way to package such "intimate" code - we can put it 
>> into String itself and just call it from StringJoiner.
>> This PR is an attempt at doing just that. It introduces new package-private 
>> method in `java.lang.String` which is then used from both pubic static 
>> `String.join` methods as well as from `java.util.StringJoiner` (via 
>> SharedSecrets). The improvements can be seen by running the following JMH 
>> benchmark:
>> 
>> https://gist.github.com/plevart/86ac7fc6d4541dbc08256cde544019ce
>> 
>> The comparative results are here:
>> 
>> https://jmh.morethan.io/?gist=7eb421cf7982456a2962269137f71c15
>> 
>> The jmh-result.json files are here:
>> 
>> https://gist.github.com/plevart/7eb421cf7982456a2962269137f71c15
>> 
>> Improvement in speed ranges from 8% (for small strings) to 200% (for long 
>> strings), while creation of garbage has been further reduced to an almost 
>> garbage-free operation.
>> 
>> So WDYT?
>
> Peter Levart has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Fix overflow checking logic, add test for it, avoid branch in loop, add 
> 1-char strings to JHM test

Looks good.

-

Marked as reviewed by rriggs (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/3501

Re: RFR: 8265237: String.join and StringJoiner can be improved further [v3]

[Peter Levart]

On Mon, 19 Apr 2021 10:44:04 GMT, Claes Redestad  wrote:

>> Peter Levart has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Fix overflow checking logic, add test for it, avoid branch in loop, add 
>> 1-char strings to JHM test
>
> test/micro/org/openjdk/bench/java/util/StringJoinerBenchmark.java line 48:
> 
>> 46: @Warmup(iterations = 10, time = 500, timeUnit = TimeUnit.MILLISECONDS)
>> 47: @Measurement(iterations = 10, time = 500, timeUnit = 
>> TimeUnit.MILLISECONDS)
>> 48: @Fork(value = 1, jvmArgsAppend = {"-Xms1g", "-Xmx1g"})
> 
> In general I think it's prudent to default to at least 3 forks, to catch 
> issues with run-to-run variance.

Thanks, Claes. Will change that to 3 forks before pushing.
Any more comments? Will wait for a day, then push it.

-

PR: https://git.openjdk.java.net/jdk/pull/3501

Integrated: 8263154: [macos] DMG builds have finder errors

[Alexander Matveev]

On Thu, 15 Apr 2021 01:24:04 GMT, Alexander Matveev  
wrote:

> - Issue was reproducible when install-dir points to some invalid location.
>  - Fixed by defaulting DMG drag and drop location to /Applications folder and 
> --install-dir will be ignored with warning for DMG.
>  - I do not see any way to support other valid, but uncommon locations for 
> drag and drop. For example: /Users/USERNAME/Applications is not possible to 
> support since user name is not known. /usr/bin requires root privileges and 
> should contain symbolic links. Locations which does not exist also not 
> possible to support, since DMG cannot create paths. So 
> /Applications/MyCompany is not possible for DMG.

This pull request has now been integrated.

Changeset: 5b43b39e
Author:Alexander Matveev 
URL:   https://git.openjdk.java.net/jdk/commit/5b43b39e
Stats: 18 lines in 6 files changed: 7 ins; 0 del; 11 mod

8263154: [macos] DMG builds have finder errors

Reviewed-by: herrick, asemenyuk

-

PR: https://git.openjdk.java.net/jdk/pull/3505

Re: RFR: 8263154: [macos] DMG builds have finder errors

[Alexey Semenyuk]

On Thu, 15 Apr 2021 01:24:04 GMT, Alexander Matveev  
wrote:

> - Issue was reproducible when install-dir points to some invalid location.
>  - Fixed by defaulting DMG drag and drop location to /Applications folder and 
> --install-dir will be ignored with warning for DMG.
>  - I do not see any way to support other valid, but uncommon locations for 
> drag and drop. For example: /Users/USERNAME/Applications is not possible to 
> support since user name is not known. /usr/bin requires root privileges and 
> should contain symbolic links. Locations which does not exist also not 
> possible to support, since DMG cannot create paths. So 
> /Applications/MyCompany is not possible for DMG.

Marked as reviewed by asemenyuk (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/3505

Re: RFR: 8263154: [macos] DMG builds have finder errors

[Andy Herrick]

On Thu, 15 Apr 2021 01:24:04 GMT, Alexander Matveev  
wrote:

> - Issue was reproducible when install-dir points to some invalid location.
>  - Fixed by defaulting DMG drag and drop location to /Applications folder and 
> --install-dir will be ignored with warning for DMG.
>  - I do not see any way to support other valid, but uncommon locations for 
> drag and drop. For example: /Users/USERNAME/Applications is not possible to 
> support since user name is not known. /usr/bin requires root privileges and 
> should contain symbolic links. Locations which does not exist also not 
> possible to support, since DMG cannot create paths. So 
> /Applications/MyCompany is not possible for DMG.

Consensus is to leave as Log.info() - so all looks good.

-

Marked as reviewed by herrick (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/3505

Re: RFR: 8265152: jpackage cleanup fails on Windows with IOException deleting msi

[Alexey Semenyuk]

On Sun, 18 Apr 2021 12:44:17 GMT, Yasumasa Suenaga  wrote:

> When creating an "exe" installer on Windows, `AbstractBundler.cleanup()` 
> calls `IOUtils.deleteRecursive()` to delete the tmp directory. It can 
> intermittently fail trying to delete the msi file.
> 
> [JDK-8263135](https://bugs.openjdk.java.net/browse/JDK-8263135) removed 
> `unique_ptr` from jpackage sources due to compiler warnings in VS 2019 
> (16.9.0), so we need to release the resource manually. However `DatabaseView` 
> didn't do that.

Marked as reviewed by asemenyuk (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/3560

Re: RFR: 4511638: Double.toString(double) sometimes produces incorrect results [v2]

[Raffaello Giulietti]

Hi Andrey,

thanks for your careful reading.

I'll keep a note and collect yours with changes coming from other 
reviewers before committing a larger batch of small changes. I would 
like to avoid wasting a lot of energy right now just to rebuild 
everything and to run the automatic tests on GitHub :-)



Greetings
Raffaello


On 2021-04-18 23:19, Andrey Turbanov wrote:

On Fri, 16 Apr 2021 11:30:32 GMT, Raffaello Giulietti 
 wrote:


Hello,

here's a PR for a patch submitted on March 2020 
[1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was a 
thing.

The patch has been edited to adhere to OpenJDK code conventions about 
multi-line (block) comments. Nothing in the code proper has changed, except for 
the addition of redundant but clarifying parentheses in some expressions.


Greetings
Raffaello


Raffaello Giulietti has updated the pull request incrementally with one 
additional commit since the last revision:

   4511638: Double.toString(double) sometimes produces incorrect results


src/java.base/share/classes/jdk/internal/math/DoubleToDecimal.java line 69:


67:
68: /* 10^(E_MIN - 1) <= MIN_VALUE < 10^E_MIN */
69: static final int E_MIN = -323;


It seems that `E_MIN`/`E_MAX`/`K_MIN`/`K_MAX`  are not used in production code.
I think it worth to move them to tests.

src/java.base/share/classes/jdk/internal/math/DoubleToDecimal.java line 117:


115:  * where there are H digits d
116:  */
117: public final int MAX_CHARS = H + 7;


Can it be made `static` ?

-

PR: https://git.openjdk.java.net/jdk/pull/3402

Re: RFR: 8265421: java/lang/String/StringRepeat.java test is missing a memory requirement [v2]

[Rahul Yadav]

On Mon, 19 Apr 2021 12:50:07 GMT, Christoph Göttschkes  wrote:

>> Please review the following patch, which fixes the mentioned test case for 
>> memory constrained devices. This can be tested on a linux based development 
>> machine, using systemd as follows:
>> 
>> 
>> $ systemd-run --user --scope -p MemoryMax=800M -p MemorySwapMax=0 
>> /usr/bin/make TEST="test/jdk/java/lang/String/StringRepeat.java" run-test
>> 
>> 
>> I split up the test case in a part which only executes the small repeat 
>> counts, and in one part which executes the huge repeat count. The second 
>> part is guarded by a requirement, so it is only executed if enough memory is 
>> available.
>
> Christoph Göttschkes has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Removes -Xmx2g for the first test case instance, which doesn't require a 
> lot of memory.

Marked as reviewed by ryadav (Committer).

-

PR: https://git.openjdk.java.net/jdk/pull/3566

Re: RFR: 8263668: Update java.time to use instanceof pattern variable [v4]

[Roger Riggs]

On Mon, 19 Apr 2021 10:38:17 GMT, Patrick Concannon  
wrote:

>> Hi,
>> 
>> Could someone please review my code for updating the code in the `java.time` 
>> package to make use of the `instanceof` pattern variable?
>> 
>> Kind regards,
>> Patrick
>
> Patrick Concannon has updated the pull request with a new target base due to 
> a merge or a rebase. The incremental webrev excludes the unrelated changes 
> brought in by the merge/rebase. The pull request contains six additional 
> commits since the last revision:
> 
>  - Merge remote-tracking branch 'origin/master' into JDK-8263668
>  - Merge remote-tracking branch 'origin/master' into JDK-8263668
>  - Merge remote-tracking branch 'origin/master' into JDK-8263668
>  - Merge remote-tracking branch 'origin/master' into JDK-8263668
>  - Merge remote-tracking branch 'origin/master' into JDK-8263668
>  - 8263668: Update java.time to use instanceof pattern variable

I generally concur with `forax` about avoiding the use of single letter 
variables.

src/java.base/share/classes/java/time/Duration.java line 1435:

> 1433: && this.seconds == other.seconds
> 1434: && this.nanos == other.nanos;
> 1435: }

Perhaps rename the argument and use `otherDuration` as the refinement.
Otherwise, an inconsistency across various classes will creep in where the more 
specific variable has a more general name.  In this case, the argument type is 
Object, so the argument name `otherDuration` is not strictly true.

src/java.base/share/classes/java/time/Instant.java line 1306:

> 1304: && this.seconds == other.seconds
> 1305: && this.nanos == other.nanos;
> 1306: }

Ditto, `otherInstance` is not strictly always an instant and it would be more 
consistent to swap the names.
`(other instanceof Instant otherInstant)`.

-

PR: https://git.openjdk.java.net/jdk/pull/3170

Re: ReversibleCollection proposal

[Stuart Marks]

On 4/17/21 9:49 AM, Remi Forax wrote:

I think the analysis is spot on but I don't think the proposed solution is the 
right one.

Introducing a new interface (two in this case) has a really huge cost and in 
this case, i've trouble to see why i will want to write a method that takes a 
ReversibleCollection as parameter, ReversibleCollection as a type does not seem 
to be useful. About the cost of introducing a new collection interface, it 
means that we also have to update all emptyXXX, singleXXX, uncheckedXXX with a 
new interface, and that only for the JDK. Every libraries that proxy 
collections has also to be updated to take care of this new type, otherwise the 
integration with an application that uses this new type and the library is not 
seamless anymore.


Hi Remi,

Thanks for looking at this. There are several issues intertwined here, so let's try 
to break it down.


There are a bunch of factory methods emptyX, singletonX, etc. that give instances of 
particular types. As Tagir noted, List is a ReversibleCollection, so you can just 
use emptyList or List.of or whatever if you need an instance of 
ReversibleCollection. Also, SortedSet/NavigableSet are ReversibleSets, so if you 
need an empty ReversibleSet, you can use Collections.emptyNavigableSet. There's no 
singletonNavigableSet, but you could do


Collections.unmodifiableNavigableSet(new TreeSet<>(List.of(x)))

which is a bit of a mouthful, but it's possible. (The fact that there's no 
Collections.singletonNavigableSet may mean this use case is so narrow that few 
people really needed it.)


As for the wrappers, synchronizedX and checkedX are mostly disused, so I don't see a 
need to provide those wrappers. Having unmodifiable RC and RS is probably useful, so 
I could see adding those. That's a certain amount of work, but not a tremendous cost.



All these methods can be introduced has default methods on the existing 
collection interfaces, there is no need of a special interface (or two) for 
that.


Clearly it's possible to get by without new interfaces. After all the collections 
framework has gotten by without them for 20+ years already. It's certainly possible 
to fill out the individual interfaces (List, Deque, Sorted/NavSet) and classes 
(LinkedHashSet, LinkedHashMap) with methods that are all similar to each other, and 
that would be useful. Or as Peter Levart pointed out, we could push all of them up 
to Collection and write the specs in a loose enough way to encompass even unordered 
collections.


Any of those alternatives (do nothing, add individual methods, add loosely-spec'd 
methods to Collection) are *possible* to do. However, I think you know how minimal 
we are with the JDK APIs, and we wouldn't have proposed new interfaces without good 
cause. Thus, I think introducing new *types* here is useful, for the following reasons.


 * There's a useful clump of semantics here, combined with sensible operations that 
rely on those semantics. There are a lot of places in the spec where there is 
hedging of the form "if the collection has an ordering, then... otherwise the 
results are undefined". This is unnecessary in the context of a new type. 
Furthermore, the new operations fit well with the new types' semantics, with no 
hedging necessary.


 * These semantics appear across a broad range of existing collection types and 
implementations. It's quite valuable to have a type that unifies the common pieces 
of List, Deque, SortedSet, and LinkedHashSet into a single abstraction.


 * It's useful to have a new type for parameters in APIs. There are times when one 
wants to accept something like a List -or- a LinkedHashSet. Typically one would 
accept a Collection and then write a spec with hedging as above ("the argument 
collection must have a defined ordering"). But this also allows bugs, for example if 
somebody accidentally passes a HashSet. Having ReversibleCollection helps this problem.


 * It's useful to have a new type for return types in APIs. Consider 
LinkedHashMap's entrySet, keySet, and values views. While we clearly get by with 
having these return Set or Collection today, we need a place to put the new methods. 
Either they go on Collection (and have extremely weak semantics) or we define new 
interfaces.


 * It's a useful interface for new collection implementations. There are data 
structures that are ordered, double-ended, and reversible, but for which 
implementing List is too onerous. Supporting int indexes in various List APIs is 
where stumbling blocks usually occur. I note that the LinkedHashMap view collections 
are examples of this that already exist in the prototype code. (Other possibilities 
might be something like SpinedBuffer or chunked linked lists.)


In general, I agree that there should be strict criteria for introducing new 
interfaces into collections, but I think this proposal meets them.


s'marks

Re: RFR: 8200559: Java agents doing instrumentation need a means to define auxiliary classes [v2]

[Alan Bateman]

On 19/04/2021 15:10, Peter Levart wrote:

:
I hear Rafael that dynamic attach is important to support monitoring and 
instrumenting large numbers of JVMs with no preparations (i.e. without issueing 
special command-line options to enable it). As I understand, current attach 
mechanism is designed to allow a process running under the same UID as the JVM 
or under root to attach to the JVM.

What if this dynamic attach mechanism was modified so that only a process 
running under root could dynamically attach to the JVM? Old behavior would be 
enabled by special command line option, so by default, dynamic attach would be 
limited to tools running under root. Rafael mentions discovery, monitoring and 
instrumenting large number of JVMs running on hosts, so if one such tool has to 
attach to different JVMs running under different UIDs, it has to run as root 
now anyway.

With such default "secure" dynamic attach and when the JVM is not running as 
root (which is a recommended security practice anyway), a library in such JVM could not 
attach back to the same JVM even through spawning sub-processes.

How to achieve such "secure" dynamic attach? One way that comes to mind 
is a modified handshake. Currently, I think at least on Linux, the tool that wishes to attach to the JVM searches for a special UNIX socket (`$PWD/.java_pid`, `/tmp/.java_pid`) and if not found, creates a special attach file (`$PWD/.attach_pid`, `/tmp/.attach_pid`) to 
signal the JVM to create a listening UNIX socket under mentioned special path, then it connects to the socket. The UNIX socket file has UID:GID set to effective UID:GID of the JVM process and permissions to 0600, so only a tool running under same UID or root can connect to such socket.


In modified handshake, JVM not running as root could not create a UNIX socket file with permissions to allow only root user to connect to it, but a tool running under root could create a listening UNIX socket with permission to allow JVM to connect to it in a way that the JVM connecting to 

such socket would know that the listening process is running as root. Simply by checking the owner of the 
listening UNIX socket file. Such socket file would have to have permission 0666 in order to allow JVMs 
running under any UID to connect to it, but otherwise be "hidden". This can be achieved by the 
tool creating a special directory and a UNIX socket in this directory, say: `/tmp/.attach_dir/`, The directory UID:GID would be 0:0 and have permission 0711. This means, 
any user could connect to the socket as long as it knows the ``, but no user but root 
can list the content of the directory to discover the name of the socket file. The last piece of the puzzle
   is how to signal to the JVM about the name of the socket file. Well, 
creating a file with the content holding the name of the socket file would be OK, as long as only target JVM could read it. File permissions could 
be set such that any process running under the same UID as the JVM could read the file. This would give a rouge library a chance to connect to the 
tool and pretend to be the monitoring JVM, but it could not connect to back to the JVM though...




One initial comment is that the attach mechanism is also used for the 
troubleshooting tools (jcmd, jstack, jmap, ...). This is how tools start 
the JMX agent in the target VM, or request a stack trace or heap dump, 
to name a few. It's just the loading of JVMTI agents that would need to 
be restricted (Java agents use the JPLIS JVMTI agent).


A second comment is that the attach mechanism on Linux and macOS is 
based on Unix domain sockets and the effective uid/gid of the peer is 
available. So if you are exploring here then you should be able to just 
use that rather than changing the handshake. The Windows implementation 
injects a stub into the target VM to queue the command so there isn't 
the equivalent.


-Alan

RFR: 8183374: Refactor java/lang/Runtime shell tests to java

[Fernando Guallini]

Refactor the following shell tests to java:
test/jdk/java/lang/RuntimeTests/shutdown/ShutdownHooks.sh
test/jdk/java/lang/Runtime/exec/SetCwd.java

In addition, the test SetCwd was running itself in separate java subprocesses 
in order to exercise Runtime.exec. It was creating a folder structure with 
multiple test class copies to distinguish between main and child processes to 
prevent an infinite recursion. That logic is simplified now, tests follow the 
testng annotations flow whereas the subprocesses entry point is a nested class 
main

-

Commit messages:
 - Merge branch 'master' of github.com:openjdk/jdk into 8183374
 - refactor ShutdownHooks and SetCwd

Changes: https://git.openjdk.java.net/jdk/pull/3572/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=3572=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8183374
  Stats: 232 lines in 4 files changed: 67 ins; 121 del; 44 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3572.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3572/head:pull/3572

PR: https://git.openjdk.java.net/jdk/pull/3572

Re: RFR: 8265418: Clean-up redundant null-checks of Class.getPackageName()

[Сергей Цыпанов]

On Mon, 19 Apr 2021 14:45:58 GMT, Alan Bateman  wrote:

>> As discussed in https://github.com/openjdk/jdk/pull/3464 we can clean-up 
>> null-checks remaining after 
>> [8142968](https://bugs.openjdk.java.net/browse/JDK-8142968) as 
>> Class.getPackageName() never returns null.
>
> src/java.base/share/classes/java/util/concurrent/atomic/AtomicIntegerFieldUpdater.java
>  line 458:
> 
>> 456: private static boolean isSamePackage(Class class1, Class 
>> class2) {
>> 457: return class1.getClassLoader() == class2.getClassLoader()
>> 458:&& 
>> class1.getPackageName().equals(class2.getPackageName());
> 
> If the j.u.c.atomic code is changed then it will need to be coordinated with 
> Doug Lea's repo. The rest of the changes are good and thanks for following up 
> from the comments in the other issue.

Hello, how should I do it? Maybe we can just mention him here?

-

PR: https://git.openjdk.java.net/jdk/pull/3571

Re: RFR: 8265418: Clean-up redundant null-checks of Class.getPackageName()

[Alan Bateman]

On Mon, 19 Apr 2021 14:05:31 GMT, Сергей Цыпанов 
 wrote:

> As discussed in https://github.com/openjdk/jdk/pull/3464 we can clean-up 
> null-checks remaining after 
> [8142968](https://bugs.openjdk.java.net/browse/JDK-8142968) as 
> Class.getPackageName() never returns null.

src/java.base/share/classes/java/util/concurrent/atomic/AtomicIntegerFieldUpdater.java
 line 458:

> 456: private static boolean isSamePackage(Class class1, Class 
> class2) {
> 457: return class1.getClassLoader() == class2.getClassLoader()
> 458:&& 
> class1.getPackageName().equals(class2.getPackageName());

If the j.u.c.atomic code is changed then it will need to be coordinated with 
Doug Lea's repo. The rest of the changes are good and thanks for following up 
from the comments in the other issue.

-

PR: https://git.openjdk.java.net/jdk/pull/3571

Integrated: 8265135: Reduce work initializing VarForms

[Claes Redestad]

On Tue, 13 Apr 2021 18:11:37 GMT, Claes Redestad  wrote:

> This patch reduces work done initializing VarForms - mostly observed when 
> loading each VarHandle implementation class.
> 
> - Lazily resolve MemberNames.
> - Streamline MethodType creation. This reduces the number of MethodTypes 
> created. 
> 
> Net effect is a reduction in bytecode executed per VH class by 50-60%.

This pull request has now been integrated.

Changeset: 5303ccb8
Author:Claes Redestad 
URL:   https://git.openjdk.java.net/jdk/commit/5303ccb8
Stats: 103 lines in 3 files changed: 54 ins; 38 del; 11 mod

8265135: Reduce work initializing VarForms

Reviewed-by: psandoz, mchung

-

PR: https://git.openjdk.java.net/jdk/pull/3472

Re: RFR: 8265135: Reduce work initializing VarForms [v5]

[Claes Redestad]

On Wed, 14 Apr 2021 21:26:52 GMT, Claes Redestad  wrote:

>> This patch reduces work done initializing VarForms - mostly observed when 
>> loading each VarHandle implementation class.
>> 
>> - Lazily resolve MemberNames.
>> - Streamline MethodType creation. This reduces the number of MethodTypes 
>> created. 
>> 
>> Net effect is a reduction in bytecode executed per VH class by 50-60%.
>
> Claes Redestad has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Add resolveOrNull for exploded MemberName

Thanks for reviewing, Mandy and Paul

-

PR: https://git.openjdk.java.net/jdk/pull/3472

Re: RFR: 8265418: Clean-up redundant null-checks of Class.getPackageName()

[Claes Redestad]

On Mon, 19 Apr 2021 14:05:31 GMT, Сергей Цыпанов 
 wrote:

> As discussed in https://github.com/openjdk/jdk/pull/3464 we can clean-up 
> null-checks remaining after 
> [8142968](https://bugs.openjdk.java.net/browse/JDK-8142968) as 
> Class.getPackageName() never returns null.

Looks good to me!

-

Marked as reviewed by redestad (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/3571

RFR: 8265418: Clean-up redundant null-checks of Class.getPackageName()

[Сергей Цыпанов]

As discussed in https://github.com/openjdk/jdk/pull/3464 we can clean-up 
null-checks remaining after 
[8142968](https://bugs.openjdk.java.net/browse/JDK-8142968) as 
Class.getPackageName() never returns null.

-

Commit messages:
 - 8265418: Clean-up redundant null-checks of Class.getPackageName()

Changes: https://git.openjdk.java.net/jdk/pull/3571/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=3571=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8265418
  Stats: 19 lines in 7 files changed: 1 ins; 8 del; 10 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3571.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3571/head:pull/3571

PR: https://git.openjdk.java.net/jdk/pull/3571

Re: RFR: 8200559: Java agents doing instrumentation need a means to define auxiliary classes [v2]

[Peter Levart]

On Mon, 19 Apr 2021 09:34:56 GMT, Alan Bateman  wrote:

> an application or library can use the attach mechanism (directly or via the 
> attach API in a child VM) to load an agent and leak the Instrumentation 
> object. This is the genie that somehow needs to be put back in its bottle. 
> One approach that I mentioned here to create is a less powerful 
> Instrumentation object for untrusted agents. Trusted agents would continue to 
> the full-power

I hear Rafael that dynamic attach is important to support monitoring and 
instrumenting large numbers of JVMs with no preparations (i.e. without issueing 
special command-line options to enable it). As I understand, current attach 
mechanism is designed to allow a process running under the same UID as the JVM 
or under root to attach to the JVM.

What if this dynamic attach mechanism was modified so that only a process 
running under root could dynamically attach to the JVM? Old behavior would be 
enabled by special command line option, so by default, dynamic attach would be 
limited to tools running under root. Rafael mentions discovery, monitoring and 
instrumenting large number of JVMs running on hosts, so if one such tool has to 
attach to different JVMs running under different UIDs, it has to run as root 
now anyway.

With such default "secure" dynamic attach and when the JVM is not running as 
root (which is a recommended security practice anyway), a library in such JVM 
could not attach back to the same JVM even through spawning sub-processes.

How to achieve such "secure" dynamic attach? One way that comes to mind is a 
modified handshake. Currently, I think at least on Linux, the tool that wishes 
to attach to the JVM searches for a special UNIX socket 
(`$PWD/.java_pid`, `/tmp/.java_pid`) and if not found, creates a 
special attach file (`$PWD/.attach_pid`, `/tmp/.attach_pid`) to 
signal the JVM to create a listening UNIX socket under mentioned special path, 
then it connects to the socket. The UNIX socket file has UID:GID set to 
effective UID:GID of the JVM process and permissions to 0600, so only a tool 
running under same UID or root can connect to such socket.

In modified handshake, JVM not running as root could not create a UNIX socket 
file with permissions to allow only root user to connect to it, but a tool 
running under root could create a listening UNIX socket with permission to 
allow JVM to connect to it in a way that the JVM connecting to such socket 
would know that the listening process is running as root. Simply by checking 
the owner of the listening UNIX socket file. Such socket file would have to 
have permission 0666 in order to allow JVMs running under any UID to connect to 
it, but otherwise be "hidden". This can be achieved by the tool creating a 
special directory and a UNIX socket in this directory, say: 
`/tmp/.attach_dir/`, The directory UID:GID would be 
0:0 and have permission 0711. This means, any user could connect to the socket 
as long as it knows the ``, but no user but root can list the 
content of the directory to discover the name of the socket file. The last 
piece of the puzzle
  is how to signal to the JVM about the name of the socket file. Well, creating 
a file with the content holding the name of the socket file would be OK, as 
long as only target JVM could read it. File permissions could be set such that 
any process running under the same UID as the JVM could read the file. This 
would give a rouge library a chance to connect to the tool and pretend to be 
the monitoring JVM, but it could not connect to back to the JVM though...

WDYT?

-

PR: https://git.openjdk.java.net/jdk/pull/3546

Re: RFR: 8265421: java/lang/String/StringRepeat.java test is missing a memory requirement [v2]

[Aleksey Shipilev]

On Mon, 19 Apr 2021 12:50:07 GMT, Christoph Göttschkes  wrote:

>> Please review the following patch, which fixes the mentioned test case for 
>> memory constrained devices. This can be tested on a linux based development 
>> machine, using systemd as follows:
>> 
>> 
>> $ systemd-run --user --scope -p MemoryMax=800M -p MemorySwapMax=0 
>> /usr/bin/make TEST="test/jdk/java/lang/String/StringRepeat.java" run-test
>> 
>> 
>> I split up the test case in a part which only executes the small repeat 
>> counts, and in one part which executes the huge repeat count. The second 
>> part is guarded by a requirement, so it is only executed if enough memory is 
>> available.
>
> Christoph Göttschkes has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Removes -Xmx2g for the first test case instance, which doesn't require a 
> lot of memory.

Marked as reviewed by shade (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/3566

Re: RFR: 8263154: [macos] DMG builds have finder errors

[Andy Herrick]

On Thu, 15 Apr 2021 01:24:04 GMT, Alexander Matveev  
wrote:

> - Issue was reproducible when install-dir points to some invalid location.
>  - Fixed by defaulting DMG drag and drop location to /Applications folder and 
> --install-dir will be ignored with warning for DMG.
>  - I do not see any way to support other valid, but uncommon locations for 
> drag and drop. For example: /Users/USERNAME/Applications is not possible to 
> support since user name is not known. /usr/bin requires root privileges and 
> should contain symbolic links. Locations which does not exist also not 
> possible to support, since DMG cannot create paths. So 
> /Applications/MyCompany is not possible for DMG.

It's my feeling that there should probably be no stdout/stderr from the 
jpackage tool other than what is specified (--help, --version, and hard 
errors).  If that is consensus we should also change the warnings about foreign 
app-image to Log.verbose, if not we can leave this Log.info() as is.

-

PR: https://git.openjdk.java.net/jdk/pull/3505

Re: RFR: 8265421: java/lang/String/StringRepeat.java test is missing a memory requirement [v2]

[Jim Laskey]

On Mon, 19 Apr 2021 12:50:07 GMT, Christoph Göttschkes  wrote:

>> Please review the following patch, which fixes the mentioned test case for 
>> memory constrained devices. This can be tested on a linux based development 
>> machine, using systemd as follows:
>> 
>> 
>> $ systemd-run --user --scope -p MemoryMax=800M -p MemorySwapMax=0 
>> /usr/bin/make TEST="test/jdk/java/lang/String/StringRepeat.java" run-test
>> 
>> 
>> I split up the test case in a part which only executes the small repeat 
>> counts, and in one part which executes the huge repeat count. The second 
>> part is guarded by a requirement, so it is only executed if enough memory is 
>> available.
>
> Christoph Göttschkes has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Removes -Xmx2g for the first test case instance, which doesn't require a 
> lot of memory.

Marked as reviewed by jlaskey (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/3566

Re: RFR: 8265152: jpackage cleanup fails on Windows with IOException deleting msi

[Andy Herrick]

On Sun, 18 Apr 2021 12:44:17 GMT, Yasumasa Suenaga  wrote:

> When creating an "exe" installer on Windows, `AbstractBundler.cleanup()` 
> calls `IOUtils.deleteRecursive()` to delete the tmp directory. It can 
> intermittently fail trying to delete the msi file.
> 
> [JDK-8263135](https://bugs.openjdk.java.net/browse/JDK-8263135) removed 
> `unique_ptr` from jpackage sources due to compiler warnings in VS 2019 
> (16.9.0), so we need to release the resource manually. However `DatabaseView` 
> didn't do that.

thanks - works like a charm

-

Marked as reviewed by herrick (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/3560

Re: RFR: 8265421: java/lang/String/StringRepeat.java test is missing a memory requirement [v2]

[Christoph Göttschkes]

> Please review the following patch, which fixes the mentioned test case for 
> memory constrained devices. This can be tested on a linux based development 
> machine, using systemd as follows:
> 
> 
> $ systemd-run --user --scope -p MemoryMax=800M -p MemorySwapMax=0 
> /usr/bin/make TEST="test/jdk/java/lang/String/StringRepeat.java" run-test
> 
> 
> I split up the test case in a part which only executes the small repeat 
> counts, and in one part which executes the huge repeat count. The second part 
> is guarded by a requirement, so it is only executed if enough memory is 
> available.

Christoph Göttschkes has updated the pull request incrementally with one 
additional commit since the last revision:

  Removes -Xmx2g for the first test case instance, which doesn't require a lot 
of memory.

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/3566/files
  - new: https://git.openjdk.java.net/jdk/pull/3566/files/e113ae08..4a2b04a7

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=3566=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=3566=00-01

  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3566.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3566/head:pull/3566

PR: https://git.openjdk.java.net/jdk/pull/3566

Re: RFR: 8265421: java/lang/String/StringRepeat.java test is missing a memory requirement [v2]

[Christoph Göttschkes]

On Mon, 19 Apr 2021 12:07:21 GMT, Aleksey Shipilev  wrote:

>> Christoph Göttschkes has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Removes -Xmx2g for the first test case instance, which doesn't require a 
>> lot of memory.
>
> test/jdk/java/lang/String/StringRepeat.java line 27:
> 
>> 25:  * @test
>> 26:  * @summary This exercises String#repeat patterns and limits.
>> 27:  * @run main/othervm -Xmx2g StringRepeat
> 
> So, does `-Xmx2g` make sense in this configuration now?

No, not really. The first test case instance should require way less than 1G of 
memory, so I will just remove the heap configuration for that instance.

-

PR: https://git.openjdk.java.net/jdk/pull/3566

Re: RFR: 8265421: java/lang/String/StringRepeat.java test is missing a memory requirement

[Aleksey Shipilev]

On Mon, 19 Apr 2021 09:42:09 GMT, Christoph Göttschkes  wrote:

> Please review the following patch, which fixes the mentioned test case for 
> memory constrained devices. This can be tested on a linux based development 
> machine, using systemd as follows:
> 
> 
> $ systemd-run --user --scope -p MemoryMax=800M -p MemorySwapMax=0 
> /usr/bin/make TEST="test/jdk/java/lang/String/StringRepeat.java" run-test
> 
> 
> I split up the test case in a part which only executes the small repeat 
> counts, and in one part which executes the huge repeat count. The second part 
> is guarded by a requirement, so it is only executed if enough memory is 
> available.

test/jdk/java/lang/String/StringRepeat.java line 27:

> 25:  * @test
> 26:  * @summary This exercises String#repeat patterns and limits.
> 27:  * @run main/othervm -Xmx2g StringRepeat

So, does `-Xmx2g` make sense in this configuration now?

-

PR: https://git.openjdk.java.net/jdk/pull/3566

Re: RFR: 8265421: java/lang/String/StringRepeat.java test is missing a memory requirement

[Jim Laskey]

On Mon, 19 Apr 2021 09:42:09 GMT, Christoph Göttschkes  wrote:

> Please review the following patch, which fixes the mentioned test case for 
> memory constrained devices. This can be tested on a linux based development 
> machine, using systemd as follows:
> 
> 
> $ systemd-run --user --scope -p MemoryMax=800M -p MemorySwapMax=0 
> /usr/bin/make TEST="test/jdk/java/lang/String/StringRepeat.java" run-test
> 
> 
> I split up the test case in a part which only executes the small repeat 
> counts, and in one part which executes the huge repeat count. The second part 
> is guarded by a requirement, so it is only executed if enough memory is 
> available.

Marked as reviewed by jlaskey (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/3566

Integrated: 8265079: Implement VarHandle invoker caching

[Jorn Vernee]

On Mon, 12 Apr 2021 16:24:37 GMT, Jorn Vernee  wrote:

> This patch implements 2 leftover TODOs for implementing var handle invoker MH 
> caching (lambda forms for those were already shared/cached).
> 
> This piggybacks on the existing mechanism for method handle invoker caching.
> 
> Testing: Local testing `java/lang/invoke` tests. Tier 1-3
> 
> Thanks,
> Jorn

This pull request has now been integrated.

Changeset: c8871487
Author:Jorn Vernee 
URL:   https://git.openjdk.java.net/jdk/commit/c8871487
Stats: 111 lines in 2 files changed: 106 ins; 0 del; 5 mod

8265079: Implement VarHandle invoker caching

Reviewed-by: redestad, vlivanov, psandoz, mchung

-

PR: https://git.openjdk.java.net/jdk/pull/3439

Re: RFR: 8265079: Implement VarHandle invoker caching [v3]

[Jorn Vernee]

On Wed, 14 Apr 2021 22:57:59 GMT, Mandy Chung  wrote:

>> Jorn Vernee has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Review comment: simplify test
>
> test/jdk/java/lang/invoke/TestVHInvokerCaching.java line 26:
> 
>> 24: /* @test
>> 25:  * @bug 8265079
>> 26:  * @run testng/othervm -Xverify:all -ea -esa TestVHInvokerCaching
> 
> Nit: the makefile to run jtreg set `-ea -esa`.   It's not strictly necessary 
> in `@run`.  If you want to run it standalone, you can run with `jtreg -ea 
> -esa` option.

Ok, I will remove them.

-

PR: https://git.openjdk.java.net/jdk/pull/3439

Re: RFR: 8265079: Implement VarHandle invoker caching [v4]

[Jorn Vernee]

> This patch implements 2 leftover TODOs for implementing var handle invoker MH 
> caching (lambda forms for those were already shared/cached).
> 
> This piggybacks on the existing mechanism for method handle invoker caching.
> 
> Testing: Local testing `java/lang/invoke` tests. Tier 1-3
> 
> Thanks,
> Jorn

Jorn Vernee has updated the pull request incrementally with one additional 
commit since the last revision:

  Remove manual -ea -esa test flags

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/3439/files
  - new: https://git.openjdk.java.net/jdk/pull/3439/files/fa5a721f..0c73b875

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=3439=03
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=3439=02-03

  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3439.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3439/head:pull/3439

PR: https://git.openjdk.java.net/jdk/pull/3439

Re: RFR: 8265237: String.join and StringJoiner can be improved further [v3]

[Claes Redestad]

On Sun, 18 Apr 2021 19:55:20 GMT, Peter Levart  wrote:

>> While JDK-8148937 improved StringJoiner class by replacing internal use of 
>> getChars that copies out characters from String elements into a char[] array 
>> with StringBuilder which is somehow more optimal, the improvement was 
>> marginal in speed (0% ... 10%) and mainly for smaller strings, while GC was 
>> reduced by about 50% in average per operation.
>> Initial attempt to tackle that issue was more involved, but was later 
>> discarded because it was apparently using too much internal String details 
>> in code that lives outside String and outside java.lang package.
>> But there is another way to package such "intimate" code - we can put it 
>> into String itself and just call it from StringJoiner.
>> This PR is an attempt at doing just that. It introduces new package-private 
>> method in `java.lang.String` which is then used from both pubic static 
>> `String.join` methods as well as from `java.util.StringJoiner` (via 
>> SharedSecrets). The improvements can be seen by running the following JMH 
>> benchmark:
>> 
>> https://gist.github.com/plevart/86ac7fc6d4541dbc08256cde544019ce
>> 
>> The comparative results are here:
>> 
>> https://jmh.morethan.io/?gist=7eb421cf7982456a2962269137f71c15
>> 
>> The jmh-result.json files are here:
>> 
>> https://gist.github.com/plevart/7eb421cf7982456a2962269137f71c15
>> 
>> Improvement in speed ranges from 8% (for small strings) to 200% (for long 
>> strings), while creation of garbage has been further reduced to an almost 
>> garbage-free operation.
>> 
>> So WDYT?
>
> Peter Levart has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Fix overflow checking logic, add test for it, avoid branch in loop, add 
> 1-char strings to JHM test

Looks good!

test/micro/org/openjdk/bench/java/util/StringJoinerBenchmark.java line 48:

> 46: @Warmup(iterations = 10, time = 500, timeUnit = TimeUnit.MILLISECONDS)
> 47: @Measurement(iterations = 10, time = 500, timeUnit = 
> TimeUnit.MILLISECONDS)
> 48: @Fork(value = 1, jvmArgsAppend = {"-Xms1g", "-Xmx1g"})

In general I think it's prudent to default to at least 3 forks, to catch issues 
with run-to-run variance.

-

Marked as reviewed by redestad (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/3501

Re: RFR: 8263668: Update java.time to use instanceof pattern variable [v4]

[Patrick Concannon]

> Hi,
> 
> Could someone please review my code for updating the code in the `java.time` 
> package to make use of the `instanceof` pattern variable?
> 
> Kind regards,
> Patrick

Patrick Concannon has updated the pull request with a new target base due to a 
merge or a rebase. The incremental webrev excludes the unrelated changes 
brought in by the merge/rebase. The pull request contains six additional 
commits since the last revision:

 - Merge remote-tracking branch 'origin/master' into JDK-8263668
 - Merge remote-tracking branch 'origin/master' into JDK-8263668
 - Merge remote-tracking branch 'origin/master' into JDK-8263668
 - Merge remote-tracking branch 'origin/master' into JDK-8263668
 - Merge remote-tracking branch 'origin/master' into JDK-8263668
 - 8263668: Update java.time to use instanceof pattern variable

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/3170/files
  - new: https://git.openjdk.java.net/jdk/pull/3170/files/d7355049..6481346a

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=3170=03
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=3170=02-03

  Stats: 69174 lines in 1873 files changed: 32339 ins; 31989 del; 4846 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3170.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3170/head:pull/3170

PR: https://git.openjdk.java.net/jdk/pull/3170

RFR: 8265036: JFR: Remove use of -XX:StartFlightRecording= and -XX:FlightRecorderOptions=

[Erik Gahlin]

Hi,

Could I have a review of fix that removes the use of "=" together with 
-XX:StartFlightRecording and -XX:FlightRecorderOptions. It's been possible to 
use "-XX:StartFlightRecording:" and "-XX:FlightRecorderOption:" since JFR was 
introduced into OpenJDK (JDK 11), so this is not a change of the specification, 
just an update to make the use consistent in tests, comments, documentation etc.

I also removed the use of -XX:+FlightRecorder, which is not needed, and has 
been deprecated since JDK 13. 

Testing: jdk/jdk/jfr, tier 1-4.

Thanks
Erik

-

Commit messages:
 - Initial

Changes: https://git.openjdk.java.net/jdk/pull/3561/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=3561=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8265036
  Stats: 97 lines in 39 files changed: 0 ins; 0 del; 97 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3561.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3561/head:pull/3561

PR: https://git.openjdk.java.net/jdk/pull/3561

Re: How to know if cpu supports unaligned memory accesses ?

[Laurent Bourgès]

Thanks, Alan !

Probably I should adopt VarHandle and use their int / long views... (for
jdk11u).

Or adapt my arrays/off-heap buffers (struct like) using primitive objects
JEP 401 for jdk17 !

It looks very promising as the Marlin renderer uses arrays of structs,
packed in arrays or off-heap (edges, rle tile cache).

Cheers,
Laurent

Le dim. 18 avr. 2021 à 09:11, Alan Bateman  a
écrit :

>
> On 17/04/2021 19:02, Laurent Bourgès wrote:
> > Hi,
> > In the Marlin renderer & in other Unsafe use cases (pixel tile
> processing)
> > for java2d pipelines, I do use putInt()/putLong() primitives even if the
> > address is not aligned (to 4 or 8 bytes) to get faster processing of byte
> > buffer or arrays...
> >
> > Is it possible in java (jdk internals) to query cpu capabilities like
> > endianness, unaligned support ?
> >
> Since Marlin is in the JDK and using Unsafe already then can't it just
> using the isBigEndian and unalignedAccess methods? The Buffer
> implementation and a few places do the same.
>
> -Alan.
>

RFR: 8265421: java/lang/String/StringRepeat.java test is missing a memory requirement

[Christoph Göttschkes]

Please review the following patch, which fixes the mentioned test case for 
memory constrained devices. This can be tested on a linux based development 
machine, using systemd as follows:


$ systemd-run --user --scope -p MemoryMax=800M -p MemorySwapMax=0 /usr/bin/make 
TEST="test/jdk/java/lang/String/StringRepeat.java" run-test


I split up the test case in a part which only executes the small repeat counts, 
and in one part which executes the huge repeat count. The second part is 
guarded by a requirement, so it is only executed if enough memory is available.

-

Commit messages:
 - Adds a second test instance which executes the huge repeat count.

Changes: https://git.openjdk.java.net/jdk/pull/3566/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=3566=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8265421
  Stats: 18 lines in 1 file changed: 13 ins; 1 del; 4 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3566.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3566/head:pull/3566

PR: https://git.openjdk.java.net/jdk/pull/3566

Re: RFR: 8200559: Java agents doing instrumentation need a means to define auxiliary classes [v2]

[Alan Bateman]

On Mon, 19 Apr 2021 09:11:34 GMT, Peter Levart  wrote:

> I think it would be easy to limit the use of this Instrumentation method to 
> the agent code as agent classes are loaded by the bootstrap classloader. 
> Simply make the method implementation caller-sensitive and check the caller 
> is from bootstrap class loader. This way Instrumentation instance escaped to 
> application code would not give that code any ability to define arbitrary 
> classes.

The agent JAR file is added to application class path and is loaded using the 
system class loader. So almost always the defining loader will be the 
application class loader.

In general it's a hard problem to try to balance the integrity and security of 
the platform with the needs of agents that do arbitrary injection and 
instrumentation. Specifying an agent on the command line with -javaagent is the 
opt-in to trust that agent and a defineClass that allows arbitrary injection is 
plausible for that deployment. As Rafael's mentioned in one of the messages, 
there is enough power in the existing Instrumentation API to do that in a round 
about way already.

We don't have anything equivalent for agents that are loaded by tools into a 
target VM. I added the attach mechanism and the dynamic agent loading back in 
JDK 6 and regret not putting more restrictions around this. As it stands, it is 
open to mis-use in that an application or library can use the attach mechanism 
(directly or via the attach API in a child VM) to load an agent and leak the 
Instrumentation object. This is the genie that somehow needs to be put back in 
its bottle. One approach that I mentioned here to create is a less powerful 
Instrumentation object for untrusted agents. Trusted agents would continue to 
the full-power Instrumentation object.  A less powerful Instrumentation object 
would not be able to redefine java.base or other core modules and would not be 
able to retransform classes in those modules. The option on the table during 
JDK 9 was to disable dynamic loading of agents by default but that was kicked 
down the road. I don't particularly like that option and I th
 ink we can do better.

-

PR: https://git.openjdk.java.net/jdk/pull/3546

Re: RFR: 8200559: Java agents doing instrumentation need a means to define auxiliary classes [v2]

[Peter Levart]

On Fri, 16 Apr 2021 20:30:15 GMT, Rafael Winterhalter 
 wrote:

>> To allow agents the definition of auxiliary classes, an API is needed to 
>> allow this. Currently, this is often achieved by using `sun.misc.Unsafe` or 
>> `jdk.internal.misc.Unsafe` ever since the `defineClass` method was removed 
>> from `sun.misc.Unsafe`.
>
> Rafael Winterhalter has refreshed the contents of this pull request, and 
> previous commits have been removed. The incremental views will show 
> differences compared to the previous content of the PR. The pull request 
> contains one new commit since the last revision:
> 
>   8200559: Java agents doing instrumentation need a means to define auxiliary 
> classes

> This won't work as agents are loaded by the system class loader, not the boot 
> loader. Peter Levart ***@***.***> schrieb am Mo., 19. Apr. 2021, 11:02:
> […](#)
> From: Alan Bateman JDK-8200559 is about defining auxiliary classes in the 
> same runtime package at load-time whereas I think the proposal in this PR is 
> adding an unrestricted defineClass to the Instrumentation class. I think this 
> will require a lot of discussion as there are significant issues and concerns 
> here. An unrestricted defineClass might be okay for tool/java agents when 
> started from the command line with -javaagent but only if the Instrumentation 
> object is never ever leaked to library or application code. It could 
> potentially be part of a large effort to reduce the capabilities of agents 
> loaded via the attach mechanism. More generally I think we need clearer 
> separation of the requirements of tool agents from the requirement of 
> framework/libraries that want to inject proxy and other classes at runtime. I 
> think it would be easy to limit the use of this Instrumentation method to the 
> agent code as agent classes are loaded by the bootstrap classloader. Simply 
> make the method imp
 lementation caller-sensitive and check the caller is from bootstrap class 
loader. This way Instrumentation instance escaped to application code would not 
give that code any ability to define arbitrary classes. Good enough? Peter 
Separately, the proposal in JEP 410 is to terminally deprecate 
ProtectionDomain. — You are receiving this because you were mentioned. Reply to 
this email directly, view it on GitHub <[#3546 
(comment)](https://github.com/openjdk/jdk/pull/3546#issuecomment-822302347)>, 
or unsubscribe 

 .

Ah sorry, I didn't know that. So what about the following: agent is able to 
define (or load) a class from bootstrap loader. So it would be able to 
instantiate an intermediary class, loaded by bootstrap loader which would serve 
as an intermediary to call into this limited Instrumentation API point...
Or better: add a Lookup parameter to the Instrumentation method. The 
implementation would check that the Lookup is an unrestricted Lookup from a 
class loaded by bootstrap loader. Agent would just have to take the pain of 
obtaining such Lookup instance...

-

PR: https://git.openjdk.java.net/jdk/pull/3546

Re: RFR: 8200559: Java agents doing instrumentation need a means to define auxiliary classes [v2]

[Peter Levart]

On Fri, 16 Apr 2021 20:30:15 GMT, Rafael Winterhalter 
 wrote:

>> To allow agents the definition of auxiliary classes, an API is needed to 
>> allow this. Currently, this is often achieved by using `sun.misc.Unsafe` or 
>> `jdk.internal.misc.Unsafe` ever since the `defineClass` method was removed 
>> from `sun.misc.Unsafe`.
>
> Rafael Winterhalter has refreshed the contents of this pull request, and 
> previous commits have been removed. The incremental views will show 
> differences compared to the previous content of the PR. The pull request 
> contains one new commit since the last revision:
> 
>   8200559: Java agents doing instrumentation need a means to define auxiliary 
> classes

From: Alan Bateman
> JDK-8200559 is about defining auxiliary classes in the same runtime package 
> at load-time whereas I think the proposal in this PR is adding an 
> unrestricted defineClass to the Instrumentation class. I think this will 
> require a lot of discussion as there are significant issues and concerns 
> here. An unrestricted defineClass might be okay for tool/java agents when 
> started from the command line with -javaagent but only if the Instrumentation 
> object is never ever leaked to library or application code. It could 
> potentially be part of a large effort to reduce the capabilities of agents 
> loaded via the attach mechanism. More generally I think we need clearer 
> separation of the requirements of tool agents from the requirement of 
> framework/libraries that want to inject proxy and other classes at runtime.

I think it would be easy to limit the use of this Instrumentation method to the 
agent code as agent classes are loaded by the bootstrap classloader. Simply 
make the method implementation caller-sensitive and check the caller is from 
bootstrap class loader. This way Instrumentation instance escaped to 
application code would not give that code any ability to define arbitrary 
classes.

Good enough?

Peter

> 
> Separately, the proposal in JEP 410 is to terminally deprecate 
> ProtectionDomain.

-

PR: https://git.openjdk.java.net/jdk/pull/3546

Re: RFR: 8200559: Java agents doing instrumentation need a means to define auxiliary classes [v2]

[Rafael Winterhalter]

On Fri, 16 Apr 2021 20:30:15 GMT, Rafael Winterhalter 
 wrote:

>> To allow agents the definition of auxiliary classes, an API is needed to 
>> allow this. Currently, this is often achieved by using `sun.misc.Unsafe` or 
>> `jdk.internal.misc.Unsafe` ever since the `defineClass` method was removed 
>> from `sun.misc.Unsafe`.
>
> Rafael Winterhalter has refreshed the contents of this pull request, and 
> previous commits have been removed. The incremental views will show 
> differences compared to the previous content of the PR. The pull request 
> contains one new commit since the last revision:
> 
>   8200559: Java agents doing instrumentation need a means to define auxiliary 
> classes

This won't work as agents are loaded by the system class loader, not the
boot loader.

Peter Levart ***@***.***> schrieb am Mo., 19. Apr. 2021,
11:02:

> From: Alan Bateman
>
> JDK-8200559 is about defining auxiliary classes in the same runtime
> package at load-time whereas I think the proposal in this PR is adding an
> unrestricted defineClass to the Instrumentation class. I think this will
> require a lot of discussion as there are significant issues and concerns
> here. An unrestricted defineClass might be okay for tool/java agents when
> started from the command line with -javaagent but only if the
> Instrumentation object is never ever leaked to library or application code.
> It could potentially be part of a large effort to reduce the capabilities
> of agents loaded via the attach mechanism. More generally I think we need
> clearer separation of the requirements of tool agents from the requirement
> of framework/libraries that want to inject proxy and other classes at
> runtime.
>
> I think it would be easy to limit the use of this Instrumentation method
> to the agent code as agent classes are loaded by the bootstrap classloader.
> Simply make the method implementation caller-sensitive and check the caller
> is from bootstrap class loader. This way Instrumentation instance escaped
> to application code would not give that code any ability to define
> arbitrary classes.
>
> Good enough?
>
> Peter
>
> Separately, the proposal in JEP 410 is to terminally deprecate
> ProtectionDomain.
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> , or
> unsubscribe
> 
> .
>

-

PR: https://git.openjdk.java.net/jdk/pull/3546