Object creation from iterating Map.of()/Set.of()/List.of()
The newer “of” methods in collections are really nice, they make creating these collections much easier and often result in better performance. However, the empty collection cases with Map.of()/Set.of()/List.of() has one small downside. The implementations within ImmutableCollections use non-specialized implementations for zero sized collections. For example, ImmutableCollections.EMPTY_MAP is a MapN. If you iterate over that Map, even if it is empty as in the case for Map.of(), a new anonymous AbstractSet is created. In comparison, Collections.emptyMap().entrySet() returns emptySet(). I don’t know what the reasoning was for rebuilding the empty based variants in ImmutableCollections. But regardless, it seems like the empty collections defined in ImmutableCollections should likewise never construct any objects. I’m happy to raise a PR to either mimic or reuse the empty collection implementations from Collections, but I wanted to check there isn’t some reasoning the of() methods work this way.
Re: List extending Collection/SequencedCollection
Hi Joe and Stuart,
Given the inconsistencies mentioned, I see how this change may not be worth the
hassle, so I’ll drop it. I appreciate the thoughtful responses to explain your
reasoning.
Thanks!
Ryan
> On Jul 7, 2023, at 4:21 PM, Stuart Marks wrote:
>
> Hi Ryan,
>
> Thanks for trying out JDK 21 early access.
>
> The issue you raise is indeed an inconsistency, but it's not clear which way
> it should be resolved, or even whether it needs to be resolved, as the
> consequences are quite minor.
>
> Specifically, when the Sequenced Collections JEP was integrated, it included
> these changes (edited for brevity):
>
> - public interface List extends Collection {
> + public interface List extends SequencedCollection {
>
> - public interface SortedSet extends Set {
> + public interface SortedSet extends Set, SequencedSet {
>
> As you observed, on List, SequencedCollection has replaced Collection, but on
> SortedSet, SequencedSet was added alongside Set. Which way is correct? Should
> SequencedCollection have been added to List, instead of replacing Collection?
> Or on SortedSet, should SequencedSet have replaced Set instead of simply
> being added?
>
> (I have to admit my first inclination was that the SortedSet change was a
> mistake, and that SequencedSet ought to have replaced Set. I think the reason
> it turned out the way it did was that, my earliest prototype, before I
> published anything, had a single interface OrderedCollection. This was
> retrofitted onto SortedSet as
>
>public interface SortedSet extends Set, OrderedCollection {
>
> Only later did I decide to add OrderedSet, and so I changed the declaration to
>
>public interface SortedSet extends Set, OrderedSet {
>
> and I didn't notice that the interfaces could be minimized. And of course
> there were a couple rounds of renaming subsequent to this.)
>
> In any case, this inconsistency is of little consequence, as the members
> available to users of the interfaces in question are the same regardless of
> the way the interfaces are declared. This is true from both a source and
> binary compatibility standpoint.
>
> Joe's point about compatibility is that even though such changes are
> *visible* to reflective code, they aren't *incompatible*. There are many
> compatible changes possible, such as moving the declaration of a method
> upward in the hierarchy. From the standpoint of reflection, that method may
> appear to have been removed from some interface; but that doesn't mean it's
> incompatible. Reflective code needs to be adjusted to take such things into
> account. The presence or absence of Collection as a superinterface of List is
> a similar case.
>
> Perhaps in some sense the JDK itself ought to be more consistent about this.
> We have for example this:
>
>class HashSet extends AbstractSet implements Set, Cloneable, Serializable
>
> but also this:
>
>class EnumSet extends AbstractSet implements Cloneable, Serializable
>
> (That is, EnumSet is "missing" Set.)
>
> Questions about this have been asked on Stack Overflow, and various answers
> there have made up a rationale about the possibly-redundant interfaces being
> included explicitly because it expresses intent more clearly, or some such.
> My own guess is that nobody has paid much attention to this issue because
> resolving it one way or another hardly makes any practical difference.
>
> s'marks
>
>> On 7/7/23 7:25 AM, Ryan Ernst wrote:
>> Thanks for laying out your thinking, Joe. I will watch your talks.
>> If I understood your response correctly, you are ok making such a change,
>> especially since it is semantically equivalent? If that’s the case, is JDK
>> 21 past the point of feature release, or should the change target only 22?
>> It doesn’t matter much to me, but I thought since SequencedCollection is
>> added in 21 it would be good to “fix” this there so as to avoid a gap in
>> behavior.
>>>> On Jun 30, 2023, at 6:13 PM, Joseph D. Darcy wrote:
>>>
>>> Hi Ryan,
>>>
>>> Apropos of this discussion, I happened to recently give a talk to the JCP
>>> that in part covered behavioral compatibility in the JDK:
>>>
>>> https://jcp.org/aboutJava/communityprocess/ec-public/materials/2023-06-13/JCP-EC-Public-Agenda-June-2023.html
>>> https://jcp.org/aboutJava/communityprocess/ec-public/materials/2023-06-13/Contributing_to_OpenJDK_2023_04_12.pdf
>>>
>>> There are many observable details of the JDK implementation, including
>>> reflective details, timing, etc. there are _not_ part of th
Re: List extending Collection/SequencedCollection
Thanks for laying out your thinking, Joe. I will watch your talks. If I understood your response correctly, you are ok making such a change, especially since it is semantically equivalent? If that’s the case, is JDK 21 past the point of feature release, or should the change target only 22? It doesn’t matter much to me, but I thought since SequencedCollection is added in 21 it would be good to “fix” this there so as to avoid a gap in behavior. > On Jun 30, 2023, at 6:13 PM, Joseph D. Darcy wrote: > > Hi Ryan, > > Apropos of this discussion, I happened to recently give a talk to the JCP > that in part covered behavioral compatibility in the JDK: > > https://jcp.org/aboutJava/communityprocess/ec-public/materials/2023-06-13/JCP-EC-Public-Agenda-June-2023.html > https://jcp.org/aboutJava/communityprocess/ec-public/materials/2023-06-13/Contributing_to_OpenJDK_2023_04_12.pdf > > There are many observable details of the JDK implementation, including > reflective details, timing, etc. there are _not_ part of the interfaces users > should rely on. > > My current evaluation here is that it would set an unfortunate precedent to > preclude making the sort of source changes in questions because of > (potential) compatibility concerns in reflective modeling, especially in a > feature release. (IMO, there is a stronger argument for not making such a > change in an update release, but even there as the change is a semantically > equivalent refactoring, I think it is generally fine.) > > HTH, > > -Joe > >> On 6/29/2023 11:30 AM, Ryan Ernst wrote: >> Thanks for replying, Joe. First, let me reiterate, we fully admit >> there was a bug in painless, we stopped short in walking the class >> hierarchy. There is no bug in the SequencedCollection hierarchy. But I >> do think there is an inconsistency. >> >>> The two definition are semantically equivalent >>> ... >>> The JDK 22 javadoc for List shows: >>>> All Superinterfaces: >>>> Collection, Iterable, SequencedCollection >> While that is true, in practice the reflection API does not give the >> same collapsed view that javadocs do. Calling getInterfaces() on a >> class only returns direct super interfaces, so >> List.class.getInterfaces() no longer returns Collection.class in JDK >> 21. >> >> The inconsistency I see is that SortedSet.class.getInterfaces() *does* >> still return Set.class. Was that intentional? It seems like either >> SortedSet does not need to extend Set directly, or List should still >> extend Collection directly. And doing the latter would provide an >> extra layer of "compatibility" for applications that may look at >> direct super interfaces, and be surprised that Collection disappeared >> across JDK versions for List. >> >>> On Wed, Jun 28, 2023 at 6:31 PM Joseph D. Darcy >>> wrote: >>> Hello, >>> >>> What is Painless doing that would fail under >>> >>> List extends SequencedCollection ... >>> >>> but work under >>> >>> List extends SequencedCollection, Collection ... >>> >>> The two definition are semantically equivalent since SequencedCollection >>> itself extends Collection. >>> >>> The JDK 22 javadoc for List shows: >>> >>>> All Superinterfaces: >>>> Collection, Iterable, SequencedCollection >>> There are certainly implementation artifacts concerning the details of >>> how a type was declared that exposed via core reflection (for the >>> javax.lang.model API during annotation processing at compile time), but >>> it is a bug for third party programs to rely on such details. >>> >>> HTH, >>> >>> -Joe >>> >>> On 6/27/2023 7:37 AM, Ryan Ernst wrote: >>>> Hi core-libs-dev, >>>> >>>> I know various threads have existed over the past few months on >>>> SequencedCollection and its implications on compatibility. I wanted to >>>> raise this semi-related issue we noticed recently after beginning >>>> testing against Java 21. >>>> >>>> Elasticsearch began testing against Java 21, and noticed a series of >>>> failures in Painless (our builtin scripting language, which >>>> dynamically compiles to bytecode). Most tests that used List failed to >>>> compile, with several unknown methods (eg add). Painless builds a >>>> hierarchy of classes it knows about for method resolution. This >>>> hierarchy didn't know anything about SequencedColl
Re: List extending Collection/SequencedCollection
Thanks for replying, Joe. First, let me reiterate, we fully admit there was a bug in painless, we stopped short in walking the class hierarchy. There is no bug in the SequencedCollection hierarchy. But I do think there is an inconsistency. > The two definition are semantically equivalent > ... > The JDK 22 javadoc for List shows: >> All Superinterfaces: >> Collection, Iterable, SequencedCollection While that is true, in practice the reflection API does not give the same collapsed view that javadocs do. Calling getInterfaces() on a class only returns direct super interfaces, so List.class.getInterfaces() no longer returns Collection.class in JDK 21. The inconsistency I see is that SortedSet.class.getInterfaces() *does* still return Set.class. Was that intentional? It seems like either SortedSet does not need to extend Set directly, or List should still extend Collection directly. And doing the latter would provide an extra layer of "compatibility" for applications that may look at direct super interfaces, and be surprised that Collection disappeared across JDK versions for List. On Wed, Jun 28, 2023 at 6:31 PM Joseph D. Darcy wrote: > > Hello, > > What is Painless doing that would fail under > > List extends SequencedCollection ... > > but work under > > List extends SequencedCollection, Collection ... > > The two definition are semantically equivalent since SequencedCollection > itself extends Collection. > > The JDK 22 javadoc for List shows: > > > All Superinterfaces: > > Collection, Iterable, SequencedCollection > > There are certainly implementation artifacts concerning the details of > how a type was declared that exposed via core reflection (for the > javax.lang.model API during annotation processing at compile time), but > it is a bug for third party programs to rely on such details. > > HTH, > > -Joe > > On 6/27/2023 7:37 AM, Ryan Ernst wrote: > > Hi core-libs-dev, > > > > I know various threads have existed over the past few months on > > SequencedCollection and its implications on compatibility. I wanted to > > raise this semi-related issue we noticed recently after beginning > > testing against Java 21. > > > > Elasticsearch began testing against Java 21, and noticed a series of > > failures in Painless (our builtin scripting language, which > > dynamically compiles to bytecode). Most tests that used List failed to > > compile, with several unknown methods (eg add). Painless builds a > > hierarchy of classes it knows about for method resolution. This > > hierarchy didn't know anything about SequencedCollection since we > > currently compile against Java 17. Now, this was ultimately a bug in > > Painless, because we knew there could be cases where we encounter > > unknown classes in the hierarchy (eg a class which is not blessed to > > be visible in the language). However, I think the reason we found that > > bug (List extending from SequencedCollection) might still cause issues > > for some. > > > > While debugging the issue, something that struck me as interesting in > > the SequencedCollection hierarchy is the difference between List and > > SortedSet. Both of these classes were made to be compatible with > > sequenced classes by adding the new classes as super interfaces, > > SequencedCollection and SequencedSet, respectively. However, while > > SortedSet was *added* as a super interface, List had its super > > interface Collection *replaced* by SequencedCollection. > > > > I don't know enough about the rampdown process to know if this is > > still fixable in Java 21. It is probably an extreme edge case that > > doesn't matter, since eg instanceof Collection will still work in > > existing code. But for consistency, it would seem better to maintain > > Collection as a direct super interface of List. Is there any reason > > such a change doesn't fit with the collections architecture going > > forward?
List extending Collection/SequencedCollection
Hi core-libs-dev, I know various threads have existed over the past few months on SequencedCollection and its implications on compatibility. I wanted to raise this semi-related issue we noticed recently after beginning testing against Java 21. Elasticsearch began testing against Java 21, and noticed a series of failures in Painless (our builtin scripting language, which dynamically compiles to bytecode). Most tests that used List failed to compile, with several unknown methods (eg add). Painless builds a hierarchy of classes it knows about for method resolution. This hierarchy didn't know anything about SequencedCollection since we currently compile against Java 17. Now, this was ultimately a bug in Painless, because we knew there could be cases where we encounter unknown classes in the hierarchy (eg a class which is not blessed to be visible in the language). However, I think the reason we found that bug (List extending from SequencedCollection) might still cause issues for some. While debugging the issue, something that struck me as interesting in the SequencedCollection hierarchy is the difference between List and SortedSet. Both of these classes were made to be compatible with sequenced classes by adding the new classes as super interfaces, SequencedCollection and SequencedSet, respectively. However, while SortedSet was *added* as a super interface, List had its super interface Collection *replaced* by SequencedCollection. I don't know enough about the rampdown process to know if this is still fixable in Java 21. It is probably an extreme edge case that doesn't matter, since eg instanceof Collection will still work in existing code. But for consistency, it would seem better to maintain Collection as a direct super interface of List. Is there any reason such a change doesn't fit with the collections architecture going forward?
Integrated: 8297451: ProcessHandleImpl should assert privilege when modifying reaper thread
On Wed, 23 Nov 2022 05:01:40 GMT, Ryan Ernst wrote: > This commit guards thread modifications for the process reaper thread with > doPrivileged. This pull request has now been integrated. Changeset: 50f9043c Author: Ryan Ernst Committer: Chris Hegarty URL: https://git.openjdk.org/jdk/commit/50f9043c6965360c426b187e47c49c42481a2549 Stats: 36 lines in 2 files changed: 29 ins; 0 del; 7 mod 8297451: ProcessHandleImpl should assert privilege when modifying reaper thread Reviewed-by: chegar, alanb - PR: https://git.openjdk.org/jdk/pull/11309
Re: RFR: 8297451: ProcessHandleImpl should assert privilege when modifying reaper thread [v3]
On Sat, 26 Nov 2022 17:24:02 GMT, Alan Bateman wrote: > Not important but can eliminate the casts from privilegedThreadSetXXX methods > if you separate the creation of the PrivilegedAction from the doPriv call. I chose to keep it as is since there was already another doPriv in the file that uses the cast style. - PR: https://git.openjdk.org/jdk/pull/11309
Re: RFR: 8297451: ProcessHandleImpl should assert privilege when modifying reaper thread [v3]
> This commit guards thread modifications for the process reaper thread with > doPrivileged. Ryan Ernst has updated the pull request incrementally with one additional commit since the last revision: Revert factory method - Changes: - all: https://git.openjdk.org/jdk/pull/11309/files - new: https://git.openjdk.org/jdk/pull/11309/files/a822cc8e..bc42d415 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=11309&range=02 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=11309&range=01-02 Stats: 29 lines in 2 files changed: 9 ins; 12 del; 8 mod Patch: https://git.openjdk.org/jdk/pull/11309.diff Fetch: git fetch https://git.openjdk.org/jdk pull/11309/head:pull/11309 PR: https://git.openjdk.org/jdk/pull/11309
Re: RFR: 8297451: ProcessHandleImpl should assert privilege when modifying reaper thread [v2]
> This commit guards thread modifications for the process reaper thread with > doPrivileged. Ryan Ernst has updated the pull request incrementally with two additional commits since the last revision: - Merge pull request #1 from ChrisHegarty/reaper_thread_modify Add privileged helper method and update existing test - Add privileged helper method and update existing test - Changes: - all: https://git.openjdk.org/jdk/pull/11309/files - new: https://git.openjdk.org/jdk/pull/11309/files/c02f3f09..a822cc8e Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=11309&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=11309&range=00-01 Stats: 31 lines in 2 files changed: 20 ins; 6 del; 5 mod Patch: https://git.openjdk.org/jdk/pull/11309.diff Fetch: git fetch https://git.openjdk.org/jdk pull/11309/head:pull/11309 PR: https://git.openjdk.org/jdk/pull/11309
RFR: 8297451: ProcessHandleImpl should assert privilege when modifying reaper thread
This commit guards thread modifications for the process reaper thread with doPrivileged. - Commit messages: - 8297451: ProcessHandleImpl should assert privilege when modifying reaper thread Changes: https://git.openjdk.org/jdk/pull/11309/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=11309&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8297451 Stats: 31 lines in 2 files changed: 19 ins; 1 del; 11 mod Patch: https://git.openjdk.org/jdk/pull/11309.diff Fetch: git fetch https://git.openjdk.org/jdk pull/11309/head:pull/11309 PR: https://git.openjdk.org/jdk/pull/11309
Integrated: 8290504: Close streams returned by ModuleReader::list
On Tue, 19 Jul 2022 14:07:17 GMT, Ryan Ernst wrote: > This commit ensures streams returned by ModuleReader::list are closed. This pull request has now been integrated. Changeset: 80bd8c35 Author: Ryan Ernst Committer: Chris Hegarty URL: https://git.openjdk.org/jdk/commit/80bd8c35494c85491963d590e7b78ea499fb691d Stats: 20 lines in 4 files changed: 11 ins; 0 del; 9 mod 8290504: Close streams returned by ModuleReader::list Reviewed-by: mchung, chegar - PR: https://git.openjdk.org/jdk/pull/9557
Re: RFR: 8290504: Close streams returned by ModuleReader::list [v4]
> This commit ensures streams returned by ModuleReader::list are closed. Ryan Ernst has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains six commits: - Merge branch 'master' into try_files/module_reader_uses - fix caller sensitive test uses - silly spaces - Merge branch 'master' into try_files/module_reader_uses - revert CallerSensitiveAccess change - 8290504: Close streams returned by ModuleReader::list This commit ensures streams returned by ModuleReader::list are closed. - Changes: https://git.openjdk.org/jdk/pull/9557/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=9557&range=03 Stats: 20 lines in 4 files changed: 11 ins; 0 del; 9 mod Patch: https://git.openjdk.org/jdk/pull/9557.diff Fetch: git fetch https://git.openjdk.org/jdk pull/9557/head:pull/9557 PR: https://git.openjdk.org/jdk/pull/9557
Re: RFR: 8290504: Close streams returned by ModuleReader::list [v4]
On Thu, 21 Jul 2022 06:53:53 GMT, Chris Hegarty wrote: >> Done in >> [4a94c64](https://github.com/openjdk/jdk/pull/9557/commits/4a94c645fe1e37abc694f81fd8e401c5dc367d68) > > @rjernst it seems that the _revert_ part of the above has been done, but not > the "have the callerSensitiveMethods DataProvider close the returned stream". Sorry I misunderstood. Fixed now in [3080378](https://github.com/openjdk/jdk/pull/9557/commits/30803780ca0bd1bcf03b99947ebf8cd4b42e6070). - PR: https://git.openjdk.org/jdk/pull/9557
Integrated: 8290359: Ensure that all directory streams are closed in jdk.link
On Fri, 15 Jul 2022 16:18:17 GMT, Ryan Ernst wrote: > This commit adds try-with-resources for uses of Stream from Files > methods that walk directories. This pull request has now been integrated. Changeset: 3582fd9e Author: Ryan Ernst Committer: Chris Hegarty URL: https://git.openjdk.org/jdk/commit/3582fd9e93d9733c6fdf1f3848e0a093d44f6865 Stats: 69 lines in 3 files changed: 17 ins; 4 del; 48 mod 8290359: Ensure that all directory streams are closed in jdk.link Reviewed-by: chegar - PR: https://git.openjdk.org/jdk/pull/9520
Integrated: 8290316: Ensure that all directory streams are closed in java.base
On Fri, 15 Jul 2022 16:06:21 GMT, Ryan Ernst wrote: > This commit guards uses of Files methods returning path streams in > java.base to use try-with-resources. This pull request has now been integrated. Changeset: 53fc495e Author: Ryan Ernst Committer: Chris Hegarty URL: https://git.openjdk.org/jdk/commit/53fc495e3aca7d89af697639d727051fb9adf9c7 Stats: 38 lines in 5 files changed: 8 ins; 3 del; 27 mod 8290316: Ensure that all directory streams are closed in java.base Reviewed-by: chegar - PR: https://git.openjdk.org/jdk/pull/9518
Integrated: 8290353: ModuleReader::list specification should suggest closing the returned stream
On Mon, 18 Jul 2022 14:06:00 GMT, Ryan Ernst wrote: > This commit adds additional clarification to the javadocs of > ModuleReader::list about needing to close the stream. The new wording is > similar to that used in Files::find and other similar methods. This pull request has now been integrated. Changeset: db1e44c2 Author:Ryan Ernst Committer: Chris Hegarty URL: https://git.openjdk.org/jdk/commit/db1e44c2ddd5b5f0db07dfc55f34fb03132f7726 Stats: 9 lines in 1 file changed: 8 ins; 0 del; 1 mod 8290353: ModuleReader::list specification should suggest closing the returned stream Reviewed-by: chegar, mchung, mr, jpai - PR: https://git.openjdk.org/jdk/pull/9538
Re: RFR: 8290504: Close streams returned by ModuleReader::list [v3]
> This commit ensures streams returned by ModuleReader::list are closed. Ryan Ernst has updated the pull request incrementally with one additional commit since the last revision: silly spaces - Changes: - all: https://git.openjdk.org/jdk/pull/9557/files - new: https://git.openjdk.org/jdk/pull/9557/files/d09846fa..b041233a Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=9557&range=02 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=9557&range=01-02 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/9557.diff Fetch: git fetch https://git.openjdk.org/jdk pull/9557/head:pull/9557 PR: https://git.openjdk.org/jdk/pull/9557
Re: RFR: 8290504: Close streams returned by ModuleReader::list [v2]
On Wed, 20 Jul 2022 20:17:27 GMT, Mandy Chung wrote:
>> test/jdk/java/lang/invoke/callerSensitive/CallerSensitiveAccess.java line
>> 411:
>>
>>> 409: try (ModuleReader reader = mref.open();
>>> 410: Stream stream = reader.list()) {
>>> 411: return stream
>>
>> This change is causing the test to fail, in the `callerSensitiveMethods`
>> DataProvider, because the data provider is expecting an open stream to be
>> returned by `callerSensitiveMethods(Module)` - the stream is now closed.
>>
>> There are a couple of ways to resolve this, but the most straightforward
>> would be to revert this part of the change, and have the
>> `callerSensitiveMethods` DataProvider close the returned stream. E.g.:
>>
>>
>> @DataProvider(name = "callerSensitiveMethods")
>> static Object[][] callerSensitiveMethods() {
>> try (var methodStream =
>> callerSensitiveMethods(Object.class.getModule())) {
>> return methodStream
>> .map(m -> new Object[]{m, shortDescription(m)})
>> .toArray(Object[][]::new);
>> }
>> }
>
> yes, this would solve it.
Done in
[4a94c64](https://github.com/openjdk/jdk/pull/9557/commits/4a94c645fe1e37abc694f81fd8e401c5dc367d68)
-
PR: https://git.openjdk.org/jdk/pull/9557
Re: RFR: 8290504: Close streams returned by ModuleReader::list [v2]
> This commit ensures streams returned by ModuleReader::list are closed. Ryan Ernst 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 three additional commits since the last revision: - Merge branch 'master' into try_files/module_reader_uses - revert CallerSensitiveAccess change - 8290504: Close streams returned by ModuleReader::list This commit ensures streams returned by ModuleReader::list are closed. - Changes: - all: https://git.openjdk.org/jdk/pull/9557/files - new: https://git.openjdk.org/jdk/pull/9557/files/3fc8cca8..d09846fa Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=9557&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=9557&range=00-01 Stats: 5022 lines in 109 files changed: 3388 ins; 1183 del; 451 mod Patch: https://git.openjdk.org/jdk/pull/9557.diff Fetch: git fetch https://git.openjdk.org/jdk pull/9557/head:pull/9557 PR: https://git.openjdk.org/jdk/pull/9557
Re: RFR: 8290353: ModuleReader::list specification should suggest closing the returned stream [v4]
> This commit adds additional clarification to the javadocs of > ModuleReader::list about needing to close the stream. The new wording is > similar to that used in Files::find and other similar methods. Ryan Ernst 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 seven additional commits since the last revision: - Merge branch 'master' into try_files/module_reader - update copyright - Merge branch 'master' into try_files/module_reader - Revert "fix uses of list() in java.base" This reverts commit a72a5b9ec4e22ca4a414554b722042d4cbfaef58. - adjust wording - fix uses of list() in java.base - 8290353: Clarify the streams returned by ModuleReader::list This commit adds additional clarification to the javadocs of ModuleReader::list about needing to close the stream. The new wording is similar to that used in Files::find and other similar methods. - Changes: - all: https://git.openjdk.org/jdk/pull/9538/files - new: https://git.openjdk.org/jdk/pull/9538/files/181c47a1..58ac453a Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=9538&range=03 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=9538&range=02-03 Stats: 5020 lines in 109 files changed: 3388 ins; 1182 del; 450 mod Patch: https://git.openjdk.org/jdk/pull/9538.diff Fetch: git fetch https://git.openjdk.org/jdk pull/9538/head:pull/9538 PR: https://git.openjdk.org/jdk/pull/9538
Re: RFR: 8290504: Close streams returned by ModuleReader::list
On Tue, 19 Jul 2022 14:07:17 GMT, Ryan Ernst wrote: > This commit ensures streams returned by ModuleReader::list are closed. Note that ModulePatcher::list delegates to ModuleReader::list, but since the stream it builds closes the underlying stream, the use doesn't need to be closed within the list method. - PR: https://git.openjdk.org/jdk/pull/9557
RFR: 8290504: Close streams returned by ModuleReader::list
This commit ensures streams returned by ModuleReader::list are closed. - Commit messages: - 8290504: Close streams returned by ModuleReader::list Changes: https://git.openjdk.org/jdk/pull/9557/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=9557&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8290504 Stats: 19 lines in 5 files changed: 10 ins; 0 del; 9 mod Patch: https://git.openjdk.org/jdk/pull/9557.diff Fetch: git fetch https://git.openjdk.org/jdk pull/9557/head:pull/9557 PR: https://git.openjdk.org/jdk/pull/9557
Re: RFR: 8290353: ModuleReader::list specification should suggest closing the returned stream [v3]
> This commit adds additional clarification to the javadocs of > ModuleReader::list about needing to close the stream. The new wording is > similar to that used in Files::find and other similar methods. Ryan Ernst 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 five additional commits since the last revision: - Merge branch 'master' into try_files/module_reader - Revert "fix uses of list() in java.base" This reverts commit a72a5b9ec4e22ca4a414554b722042d4cbfaef58. - adjust wording - fix uses of list() in java.base - 8290353: Clarify the streams returned by ModuleReader::list This commit adds additional clarification to the javadocs of ModuleReader::list about needing to close the stream. The new wording is similar to that used in Files::find and other similar methods. - Changes: - all: https://git.openjdk.org/jdk/pull/9538/files - new: https://git.openjdk.org/jdk/pull/9538/files/0b3ca182..181c47a1 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=9538&range=02 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=9538&range=01-02 Stats: 1282 lines in 88 files changed: 948 ins; 133 del; 201 mod Patch: https://git.openjdk.org/jdk/pull/9538.diff Fetch: git fetch https://git.openjdk.org/jdk pull/9538/head:pull/9538 PR: https://git.openjdk.org/jdk/pull/9538
Re: RFR: 8290353: Clarify the streams returned by ModuleReader::list [v2]
> This commit adds additional clarification to the javadocs of > ModuleReader::list about needing to close the stream. The new wording is > similar to that used in Files::find and other similar methods. Ryan Ernst has updated the pull request incrementally with two additional commits since the last revision: - adjust wording - fix uses of list() in java.base - Changes: - all: https://git.openjdk.org/jdk/pull/9538/files - new: https://git.openjdk.org/jdk/pull/9538/files/6c9750f7..0b3ca182 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=9538&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=9538&range=00-01 Stats: 23 lines in 6 files changed: 10 ins; 0 del; 13 mod Patch: https://git.openjdk.org/jdk/pull/9538.diff Fetch: git fetch https://git.openjdk.org/jdk pull/9538/head:pull/9538 PR: https://git.openjdk.org/jdk/pull/9538
Re: RFR: 8290359: Ensure that all directory streams are closed in jdk.link [v2]
On Mon, 18 Jul 2022 15:35:25 GMT, Chris Hegarty wrote:
>> Ryan Ernst 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 three additional
>> commits since the last revision:
>>
>> - Merge branch 'master' into try_files/jdk.link
>> - fix alignment
>> - 8290359: Ensure that all directory streams are closed in jdk.link
>>
>>This commit adds try-with-resources for uses of Stream from Files
>>methods that walk directories.
>
> src/jdk.jlink/share/classes/jdk/tools/jlink/internal/JlinkTask.java line 834:
>
>> 832: name.endsWith(".EC") ||
>> 833: name.startsWith("META-INF/SIG-")
>> 834: );
>
> Trivially, can we please keep the indentation consistent with the previous
> version. So, align all `name.endsWith` expressions under the 's' from
> startsWith.
Done in
[c628479](https://github.com/openjdk/jdk/pull/9520/commits/c62847976c4a34ee97be61bbac3002513028e87c)
-
PR: https://git.openjdk.org/jdk/pull/9520
Re: RFR: 8290359: Ensure that all directory streams are closed in jdk.link [v2]
> This commit adds try-with-resources for uses of Stream from Files > methods that walk directories. Ryan Ernst 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 three additional commits since the last revision: - Merge branch 'master' into try_files/jdk.link - fix alignment - 8290359: Ensure that all directory streams are closed in jdk.link This commit adds try-with-resources for uses of Stream from Files methods that walk directories. - Changes: - all: https://git.openjdk.org/jdk/pull/9520/files - new: https://git.openjdk.org/jdk/pull/9520/files/f5d4fd2e..4e9eede0 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=9520&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=9520&range=00-01 Stats: 1454 lines in 71 files changed: 802 ins; 379 del; 273 mod Patch: https://git.openjdk.org/jdk/pull/9520.diff Fetch: git fetch https://git.openjdk.org/jdk pull/9520/head:pull/9520 PR: https://git.openjdk.org/jdk/pull/9520
Re: RFR: 8290353: Clarify the streams returned by ModuleReader::list
On Mon, 18 Jul 2022 14:06:00 GMT, Ryan Ernst wrote: > This commit adds additional clarification to the javadocs of > ModuleReader::list about needing to close the stream. The new wording is > similar to that used in Files::find and other similar methods. I relaxed the wording in [0b3ca18](https://github.com/openjdk/jdk/pull/9538/commits/0b3ca18267b4a54db7d749dba036b4618f2eb0e6) and addressed uses of list() in java.base in [a72a5b9](https://github.com/openjdk/jdk/pull/9538/commits/a72a5b9ec4e22ca4a414554b722042d4cbfaef58). - PR: https://git.openjdk.org/jdk/pull/9538
RFR: 8290353: Clarify the streams returned by ModuleReader::list
This commit adds additional clarification to the javadocs of ModuleReader::list about needing to close the stream. The new wording is similar to that used in Files::find and other similar methods. - Commit messages: - 8290353: Clarify the streams returned by ModuleReader::list Changes: https://git.openjdk.org/jdk/pull/9538/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=9538&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8290353 Stats: 8 lines in 1 file changed: 8 ins; 0 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/9538.diff Fetch: git fetch https://git.openjdk.org/jdk pull/9538/head:pull/9538 PR: https://git.openjdk.org/jdk/pull/9538
Re: RFR: 8290316: Ensure that all directory streams are closed in java.base [v3]
> This commit guards uses of Files methods returning path streams in > java.base to use try-with-resources. Ryan Ernst has updated the pull request incrementally with one additional commit since the last revision: fix compile and address more feedback - Changes: - all: https://git.openjdk.org/jdk/pull/9518/files - new: https://git.openjdk.org/jdk/pull/9518/files/c0a09f91..b5076b1a Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=9518&range=02 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=9518&range=01-02 Stats: 4 lines in 2 files changed: 1 ins; 0 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/9518.diff Fetch: git fetch https://git.openjdk.org/jdk pull/9518/head:pull/9518 PR: https://git.openjdk.org/jdk/pull/9518
Re: RFR: 8290316: Ensure that all directory streams are closed in java.base
On Fri, 15 Jul 2022 16:06:21 GMT, Ryan Ernst wrote: > This commit guards uses of Files methods returning path streams in > java.base to use try-with-resources. Thanks Alan. The 8 space indents were unintentional. I think I've addressed all your comments in [c0a09f9](https://github.com/openjdk/jdk/pull/9518/commits/c0a09f91be22acce0555e5a8d06023975185d307). - PR: https://git.openjdk.org/jdk/pull/9518
Re: RFR: 8290316: Ensure that all directory streams are closed in java.base [v2]
> This commit guards uses of Files methods returning path streams in > java.base to use try-with-resources. Ryan Ernst has updated the pull request incrementally with one additional commit since the last revision: address formatting feedback - Changes: - all: https://git.openjdk.org/jdk/pull/9518/files - new: https://git.openjdk.org/jdk/pull/9518/files/bba7406f..c0a09f91 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=9518&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=9518&range=00-01 Stats: 25 lines in 4 files changed: 3 ins; 4 del; 18 mod Patch: https://git.openjdk.org/jdk/pull/9518.diff Fetch: git fetch https://git.openjdk.org/jdk pull/9518/head:pull/9518 PR: https://git.openjdk.org/jdk/pull/9518
RFR: 8290359: Ensure that all directory streams are closed in jdk.link
This commit adds try-with-resources for uses of Stream from Files methods that walk directories. - Commit messages: - 8290359: Ensure that all directory streams are closed in jdk.link Changes: https://git.openjdk.org/jdk/pull/9520/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=9520&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8290359 Stats: 73 lines in 3 files changed: 17 ins; 4 del; 52 mod Patch: https://git.openjdk.org/jdk/pull/9520.diff Fetch: git fetch https://git.openjdk.org/jdk pull/9520/head:pull/9520 PR: https://git.openjdk.org/jdk/pull/9520
RFR: 8290316: Ensure that all directory streams are closed in java.base
This commit guards uses of Files methods returning path streams in java.base to use try-with-resources. - Commit messages: - 8290316: Ensure that all directory streams are closed in java.base Changes: https://git.openjdk.org/jdk/pull/9518/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=9518&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8290316 Stats: 52 lines in 5 files changed: 12 ins; 7 del; 33 mod Patch: https://git.openjdk.org/jdk/pull/9518.diff Fetch: git fetch https://git.openjdk.org/jdk pull/9518/head:pull/9518 PR: https://git.openjdk.org/jdk/pull/9518
Re: RFR: 8288984: Simplification in Shutdown.exit [v2]
On Mon, 4 Jul 2022 16:47:28 GMT, Chris Hegarty wrote: >> I think the wording in the latest commit (9d972b) is problematic. One reason >> is that you've changed it to "will run shutdown hooks" but it doesn't run >> the hooks, it starts them, and so conflicts with the previous paragraph. I >> also think "That invocation may be initiated via platform specific signal >> handlers" is confusing here as there is no notion of "signal handler" to >> reference. There may be scope for text elsewhere, maybe with an implNote for >> signals such as HUP, but I don't think this is the PR for this. So I think >> it better to try the wording that David suggested and see if it needs any >> improvement. > > YAO (Yet Another Opinion)! But I think we're actually converging. > > David's wording: >>Invocations of this method are serialized such that only one invocation will >>actually proceed with the shutdown sequence and terminate the VM with the >>given status code. > > ... gives the impression that an invocation of this method WILL terminate the > VM with the given status code - which is not actually true, given the > potential for signals. This is already alluded to in > `Runtime::addShutdownHook`. I agree that this could be resolved separately, > but we should _allow_ for signals (even if we don't explicitly mention them). > > The original wording: >> If this method is invoked after all shutdown hooks have already been run ... > > ... seems quite clever (and allows for signals). Maybe we could keep things > super simple: >> If this method is invoked after shutdown hooks have already been started, it >> will block indefinitely. If this method is invoked from a shutdown hook the >> system will deadlock. I like the new suggested wording, but I would slightly tweak it. As Alan mentioned in another comment the first paragraph makes clear the method never returns normally. How does the following sound? > If this method is invoked after shutdown hooks have already been started, the > supplied status code will be ignored. If this method is invoked from a > shutdown hook the system will deadlock. - PR: https://git.openjdk.org/jdk/pull/9351
Re: RFR: 8288984: Simplification in java.lang.Runtime::exit [v4]
> This is a followup to simplify Shutdown.exit after the removal of > finalizers (https://bugs.openjdk.org/browse/JDK-8198250). Once agreement > on the approach has been reached in this PR, a CSR will be filed to > propose the spec change to Runtime.exit. Ryan Ernst has updated the pull request incrementally with one additional commit since the last revision: iter text - Changes: - all: https://git.openjdk.org/jdk/pull/9351/files - new: https://git.openjdk.org/jdk/pull/9351/files/2253259c..fccd85ba Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=9351&range=03 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=9351&range=02-03 Stats: 6 lines in 1 file changed: 0 ins; 1 del; 5 mod Patch: https://git.openjdk.org/jdk/pull/9351.diff Fetch: git fetch https://git.openjdk.org/jdk pull/9351/head:pull/9351 PR: https://git.openjdk.org/jdk/pull/9351
Re: RFR: 8288984: Simplification in Shutdown.exit [v3]
On Tue, 5 Jul 2022 04:06:03 GMT, David Holmes wrote: >> Signal handlers for eg SIGTERM invoke Shutdown.shutdown. That method holds >> the same lock as Shutdown.exit and runs shutdown hooks. So if a signal >> handler triggers shutdown, and before the system halts Runtime.exit is >> invoked, the status passed to Runtime.exit will be ignored. > > First, the signal handlers actually invoke Shutdown.exit, not > Shutdown.shutdown - see Terminator.setup(). (If they did the latter, like > DestroyJavaVM, then an exit(status) call from another thread could still do > the final VM halt(status) > > But now you are opening a can of worms. There are multiple ways for the VM to > initiate termination - are you going to try and describe here how all of them > potentially interact? > > What you are really stating here is that other parts of the JDK can invoke > System.exit, but that is for them to specify where such things are specified, > it isn't for exit() to try and list them all. All we have to do here is > describe how multiple calls to exit() behave. Aha! I think I misread the comment on Shutdown.shutdown and had it stuck in my head that signals would exit through that method. Thanks for the explanation, it makes a lot more sense now. > All we have to do here is describe how multiple calls to exit() behave. The thing that is still missing to me is that an application developer may have a single call to System.exit, yet the status code they pass to it may not be the one the jvm exits with. - PR: https://git.openjdk.org/jdk/pull/9351
Re: RFR: 8288984: Simplification in Shutdown.exit [v3]
On Tue, 5 Jul 2022 00:16:50 GMT, David Holmes wrote: >> Ryan Ernst has updated the pull request incrementally with one additional >> commit since the last revision: >> >> iterate on wording > > src/java.base/share/classes/java/lang/Runtime.java line 88: > >> 86: * Shutdown is serialized such that only one invocation will run >> 87: * shutdown hooks and terminate the VM with the given status code. >> That >> 88: * invocation may be initiated via platform specific signal >> handlers. All > > Why are we mentioning signal handlers here? How is that relevant? Signal handlers for eg SIGTERM invoke Shutdown.shutdown. That method holds the same lock as Shutdown.exit and runs shutdown hooks. So if a signal handler triggers shutdown, and before the system halts Runtime.exit is invoked, the status passed to Runtime.exit will be ignored. - PR: https://git.openjdk.org/jdk/pull/9351
Re: RFR: 8288984: Simplification in java.lang.Runtime::exit [v5]
> This is a followup to simplify Shutdown.exit after the removal of > finalizers (https://bugs.openjdk.org/browse/JDK-8198250). Once agreement > on the approach has been reached in this PR, a CSR will be filed to > propose the spec change to Runtime.exit. Ryan Ernst 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 five additional commits since the last revision: - Merge branch 'master' into shutdown_cleanup - iter text - iterate on wording - better clarify multiple threads behavior - 8288984: Simplification in Shutdown.exit This is a followup to simplify Shutdown.exit after the removal of finalizers (https://bugs.openjdk.org/browse/JDK-8198250). Once agreement on the approach has been reached in this PR, a CSR will be filed to propose the spec change to Runtime.exit. - Changes: - all: https://git.openjdk.org/jdk/pull/9351/files - new: https://git.openjdk.org/jdk/pull/9351/files/fccd85ba..75a2651e Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=9351&range=04 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=9351&range=03-04 Stats: 2278 lines in 110 files changed: 1285 ins; 604 del; 389 mod Patch: https://git.openjdk.org/jdk/pull/9351.diff Fetch: git fetch https://git.openjdk.org/jdk pull/9351/head:pull/9351 PR: https://git.openjdk.org/jdk/pull/9351
Re: RFR: 8288984: Simplification in java.lang.Runtime::exit [v2]
On Mon, 4 Jul 2022 12:19:27 GMT, David Holmes wrote: >> Ryan Ernst has updated the pull request incrementally with one additional >> commit since the last revision: >> >> better clarify multiple threads behavior > >> Let's say we've run shutdown so run all the hooks but not halted. Then >> someone calls exit(0). That seems to suggest the call will block >> indefinitely, which is neither desirable nor what was actually implemented. > > If the hook threads do not halt then the exiting thread (which holds the > lock) blocks forever in the join(). Any other call to exit blocks trying to > acquire the lock. That has always been the way this works - if hook threads > don't terminate then the VM doesn't (unless someone calls halt() directly). > That is one of the things the window you suggested be closed, allowed - a > call to exit(non-zero) could force a call to halt(). I appreciate all the feedback and the many opinions expressed here! This has been a learning exercise for me in finding a balance between implementation and specification. While I still think mentioning the possibility of signals is beneficial to a developer trying to understand that the passed status code could be ignored, the text suggested by @dholmes-ora is better than was previously there, so I have updated this PR with that. See [fccd85b](https://github.com/openjdk/jdk/pull/9351/commits/fccd85ba106ff651c00479446ac3207ed60698e8). - PR: https://git.openjdk.org/jdk/pull/9351
Re: RFR: 8288984: Simplification in Shutdown.exit [v2]
On Mon, 4 Jul 2022 08:42:02 GMT, Alan Bateman wrote: >> +1 - except for the "deadlock" part (see other comment). I think the old >> paragraph is at least confusing, and perhaps even just wrong. Let's say >> we've run `shutdown` so run all the hooks but not halted. Then someone >> calls `exit(0)`. That seems to suggest the call will block indefinitely, >> which is neither desirable nor what was actually implemented. > > David's refinement looks good. The sentence on deadlock is accurate as > shutdown hooks run on other threads, not the thread calling System.ext. I reworded with the suggested text in mind. I also added another sentence referencing native signal handlers, which may also initiate shutdown. I think this clarification is important so that it is clear the status code of all invocations from Java may still be ignored. See [2253259](https://github.com/openjdk/jdk/pull/9351/commits/2253259c82b13e7b2186429a80cc42497235b035). - PR: https://git.openjdk.org/jdk/pull/9351
Re: RFR: 8288984: Simplification in Shutdown.exit [v3]
> This is a followup to simplify Shutdown.exit after the removal of > finalizers (https://bugs.openjdk.org/browse/JDK-8198250). Once agreement > on the approach has been reached in this PR, a CSR will be filed to > propose the spec change to Runtime.exit. Ryan Ernst has updated the pull request incrementally with one additional commit since the last revision: iterate on wording - Changes: - all: https://git.openjdk.org/jdk/pull/9351/files - new: https://git.openjdk.org/jdk/pull/9351/files/8e7d527a..2253259c Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=9351&range=02 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=9351&range=01-02 Stats: 6 lines in 1 file changed: 2 ins; 0 del; 4 mod Patch: https://git.openjdk.org/jdk/pull/9351.diff Fetch: git fetch https://git.openjdk.org/jdk pull/9351/head:pull/9351 PR: https://git.openjdk.org/jdk/pull/9351
Re: RFR: 8288984: Simplification in Shutdown.exit [v2]
On Sat, 2 Jul 2022 19:32:04 GMT, Alan Bateman wrote: >> If a shutdown hook is running, then shutdown has started, right? This new >> wording isn’t really a change, it’s the current behavior (the “otherwise” >> portion of the old wording). But the wording is meant to be more accurate >> for the case of running exit from shutdown hooks. The change is the removal >> of the first case, which is what the code portion of the PR removes. > > The first paragraph of the method description already makes it clear that the > method never returns normally. One option for the third paragraph is to just > remove it, another is to replace it with wording that specifies that the > first thread to call exit is the winner and the exit code provided by other > threads that attempt to exit around the same time (or while exit hooks > execute) will be ignored. I think there is merit with adding a warning that a > shutdown hook invoking exit will deadlock. Thanks for the feedback, they are all good points. I opted to make the behavior with multiple threads more clear, see [8e7d527](https://github.com/openjdk/jdk/pull/9351/commits/8e7d527a182933818bd2d7f0f1b799dac52663be). - PR: https://git.openjdk.org/jdk/pull/9351
Re: RFR: 8288984: Simplification in Shutdown.exit [v2]
> This is a followup to simplify Shutdown.exit after the removal of > finalizers (https://bugs.openjdk.org/browse/JDK-8198250). Once agreement > on the approach has been reached in this PR, a CSR will be filed to > propose the spec change to Runtime.exit. Ryan Ernst has updated the pull request incrementally with one additional commit since the last revision: better clarify multiple threads behavior - Changes: - all: https://git.openjdk.org/jdk/pull/9351/files - new: https://git.openjdk.org/jdk/pull/9351/files/9d972ba6..8e7d527a Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=9351&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=9351&range=00-01 Stats: 4 lines in 1 file changed: 1 ins; 0 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/9351.diff Fetch: git fetch https://git.openjdk.org/jdk pull/9351/head:pull/9351 PR: https://git.openjdk.org/jdk/pull/9351
Re: RFR: 8288984: Simplification in Shutdown.exit
On Sat, 2 Jul 2022 13:21:06 GMT, David Holmes wrote: >> This is a followup to simplify Shutdown.exit after the removal of >> finalizers (https://bugs.openjdk.org/browse/JDK-8198250). Once agreement >> on the approach has been reached in this PR, a CSR will be filed to >> propose the spec change to Runtime.exit. > > src/java.base/share/classes/java/lang/Runtime.java line 88: > >> 86: * Invocations of this method block indefinitely. It is therefore >> 87: * inadvisable to invoke this method from a shutdown hook as it will >> 88: * cause deadlock. > > This is inaccurate. The method only blocks indefinitely when shutdown has > already commenced. If a shutdown hook is running, then shutdown has started, right? This new wording isn’t really a change, it’s the current behavior (the “otherwise” portion of the old wording). But the wording is meant to be more accurate for the case of running exit from shutdown hooks. The change is the removal of the first case, which is what the code portion of the PR removes. - PR: https://git.openjdk.org/jdk/pull/9351
RFR: 8288984: Simplification in Shutdown.exit
This is a followup to simplify Shutdown.exit after the removal of finalizers (https://bugs.openjdk.org/browse/JDK-8198250). Once agreement on the approach has been reached in this PR, a CSR will be filed to propose the spec change to Runtime.exit. - Commit messages: - 8288984: Simplification in Shutdown.exit Changes: https://git.openjdk.org/jdk/pull/9351/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=9351&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8288984 Stats: 10 lines in 2 files changed: 0 ins; 7 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/9351.diff Fetch: git fetch https://git.openjdk.org/jdk pull/9351/head:pull/9351 PR: https://git.openjdk.org/jdk/pull/9351
