Re: RFR: 8324573: HashMap::putAll add notes for conservative resizing [v7]

2024-04-05 Thread Volker Simonis
On Thu, 4 Apr 2024 22:18:16 GMT, Joshua Cao wrote: > I tried looking at the docs with `make docs-image`, but I can't test that the > syntax/links are actually correct because `@implNote` does not actually show > up in the web pages. As I understand from [the original >

Re: RFR: 8324573: HashMap::putAll add notes for conservative resizing [v6]

2024-04-04 Thread Volker Simonis
On Wed, 3 Apr 2024 01:37:37 GMT, Joshua Cao wrote: >> Add notes for `HashMap::putAll()` conservative resizing. >> >> Note: everything below this line is from the original change. After >> discussion, we decided to keep the conservative resizing, but we should add >> an `@implNote` for the

Re: RFR: 8324573: HashMap::putAll add notes for conservative resizing [v5]

2024-02-29 Thread Volker Simonis
On Fri, 16 Feb 2024 23:30:22 GMT, Joshua Cao wrote: >> Add notes for `HashMap::putAll()` conservative resizing. >> >> Note: everything below this line is from the original change. After >> discussion, we decided to keep the conservative resizing, but we should add >> an `@implNote` for the

Re: RFR: 8324573: HashMap::putAll should resize to sum of both map sizes [v4]

2024-02-09 Thread Volker Simonis
On Fri, 2 Feb 2024 17:38:13 GMT, Joshua Cao wrote: >> This change mirrors what we did for ConcurrentHashMap in >> https://github.com/openjdk/jdk/pull/17116. When we add all entries from one >> map to anther, we should resize that map to the size of the sum of both maps. >> >> I used the

Re: RFR: 8324573: HashMap::putAll should resize to sum of both map sizes

2024-01-24 Thread Volker Simonis
On Wed, 24 Jan 2024 00:26:09 GMT, Joshua Cao wrote: > This change mirrors what we did for ConcurrentHashMap in > https://github.com/openjdk/jdk/pull/17116. When we add all entries from one > map to anther, we should resize that map to the size of the sum of both maps. > > I used the command

Re: RFR: 8322149: ConcurrentHashMap smarter presizing for copy constructor and putAll [v3]

2024-01-23 Thread Volker Simonis
On Mon, 22 Jan 2024 22:10:44 GMT, Joshua Cao wrote: >> test/micro/org/openjdk/bench/java/util/concurrent/Maps.java line 122: >> >>> 120: @Benchmark >>> 121: public ConcurrentHashMap >>> testConcurrentHashMapPutAll() { >>> 122: ConcurrentHashMap map = new >>>

Re: RFR: 8322149: ConcurrentHashMap smarter presizing for copy constructor and putAll [v4]

2024-01-23 Thread Volker Simonis
On Mon, 22 Jan 2024 22:13:50 GMT, Joshua Cao wrote: >> ConcurrentHashMap's copy constructor calls `putAll()` -> `tryPresize()` -> >> `transfer()`. When coming from the copy constructor, the Map is empty, so >> there is nothing to transfer. But `transfer()` will still copy all the empty >>

Re: RFR: 8322149: ConcurrentHashMap smarter presizing for copy constructor and putAll [v3]

2024-01-22 Thread Volker Simonis
On Wed, 17 Jan 2024 21:16:02 GMT, Joshua Cao wrote: >> ConcurrentHashMap's copy constructor calls `putAll()` -> `tryPresize()` -> >> `transfer()`. When coming from the copy constructor, the Map is empty, so >> there is nothing to transfer. But `transfer()` will still copy all the empty >>

Re: RFR: 8322149: ConcurrentHashMap copy constructor should not transfer from old table on presizing [v2]

2024-01-12 Thread Volker Simonis
On Thu, 11 Jan 2024 19:21:00 GMT, Joshua Cao wrote: >>> We don't need to compute max() here. >>> [tryPresize()](https://github.com/openjdk/jdk/blob/8a4dc79e1a40e7115e2971af81623b6b0368f41c/src/java.base/share/classes/java/util/concurrent/ConcurrentHashMap.java#L2397) >>> does that already. >>

Re: RFR: 8322149: ConcurrentHashMap copy constructor should not transfer from old table on presizing [v2]

2024-01-11 Thread Volker Simonis
On Mon, 8 Jan 2024 20:27:59 GMT, Joshua Cao wrote: > We don't need to compute max() here. > [tryPresize()](https://github.com/openjdk/jdk/blob/8a4dc79e1a40e7115e2971af81623b6b0368f41c/src/java.base/share/classes/java/util/concurrent/ConcurrentHashMap.java#L2397) > does that already. The code

Re: RFR: 8322149: ConcurrentHashMap copy constructor should not transfer from old table on presizing [v2]

2024-01-08 Thread Volker Simonis
On Wed, 3 Jan 2024 21:29:57 GMT, Joshua Cao wrote: >> ConcurrentHashMap's copy constructor calls `putAll()` -> `tryPresize()` -> >> `transfer()`. When coming from the copy constructor, the Map is empty, so >> there is nothing to transfer. But `transfer()` will still copy all the empty >>

Re: RFR: JDK-8319122: Improve documentation of various Zip-file related APIs

2023-11-06 Thread Volker Simonis
On Mon, 30 Oct 2023 17:26:53 GMT, Yakov Shafranovich wrote: > The various Zip/Jar-file related Java APIs have some long-standing > differences or peculiarities with respect to the ZIP-file specification or > compared to other implementations which should be documented in the API-doc. > This

Re: RFR: JDK-8319122: Improve documentation of various Zip-file related APIs

2023-11-03 Thread Volker Simonis
On Mon, 30 Oct 2023 17:26:53 GMT, Yakov Shafranovich wrote: > The various Zip/Jar-file related Java APIs have some long-standing > differences or peculiarities with respect to the ZIP-file specification or > compared to other implementations which should be documented in the API-doc. > This

Re: RFR: 8316879: RegionMatches1Tests fails if CompactStrings are disabled after JDK-8302163

2023-09-25 Thread Volker Simonis
On Mon, 25 Sep 2023 15:52:12 GMT, Aleksei Voitylov wrote: > test java.lang.String.RegionMatches1Tests fails on all platforms with > -XX:-CompactStrings option and on ARM32 where Compact Strings is disabled by > default. The fix is to return true immediately if len is negative, since for >

Re: RFR: 8316879: RegionMatches1Tests fails if CompactStrings are disabled after JDK-8302163

2023-09-25 Thread Volker Simonis
On Mon, 25 Sep 2023 15:52:12 GMT, Aleksei Voitylov wrote: > test java.lang.String.RegionMatches1Tests fails on all platforms with > -XX:-CompactStrings option and on ARM32 where Compact Strings is disabled by > default. The fix is to return true immediately if len is negative, since for >

Re: RFR: 8316879: RegionMatches1Tests fails if CompactStrings are disabled after JDK-8302163

2023-09-25 Thread Volker Simonis
On Mon, 25 Sep 2023 15:52:12 GMT, Aleksei Voitylov wrote: > test java.lang.String.RegionMatches1Tests fails on all platforms with > -XX:-CompactStrings option and on ARM32 where Compact Strings is disabled by > default. The fix is to return true immediately if len is negative, since for >

Re: RFR: 8285447: StackWalker minimal batch size should be optimized for getCallerClass [v4]

2023-09-12 Thread Volker Simonis
On Mon, 11 Sep 2023 17:52:56 GMT, Mandy Chung wrote: >> Typically it will find the caller class at the second stack frame from the >> frame of executing `StackWalker::getCallerClass`. The initial size of the >> buffer can be changed from 8 to 4 (the top 2 elements are reserved for >>

Re: RFR: 8285447: StackWalker minimal batch size should be optimized for getCallerClass

2023-09-11 Thread Volker Simonis
On Fri, 8 Sep 2023 16:57:14 GMT, Mandy Chung wrote: > Typically it will find the caller class at the second stack frame from the > frame of executing `StackWalker::getCallerClass`. The initial size of the > buffer can be changed from 8 to 4 (the top 2 elements are reserved for >

Re: Question on why sun.management MBeans are not exported?

2023-09-06 Thread Volker Simonis
On Wed, Sep 6, 2023 at 3:47 PM Alan Bateman wrote: > > On 06/09/2023 14:02, Volker Simonis wrote: > > : > > > > I wonder why "sun.management" was encapsulated in the first place? I > > understand that it is not an "officially supported" API, but

Question on why sun.management MBeans are not exported?

2023-09-06 Thread Volker Simonis
Hi, I recently looked for an easy way to get the CPU time spent by JIT-compiler and GC threads in Java (e.g exported by IBM J9's JvmCpuMonitorMXBean [0]). An easy way to achieve this is in OpenJDK is by using the "sun.management.HotspotInternal" MBean which exports the

Re: RFR: 8268829: Provide an optimized way to walk the stack with Class object only [v6]

2023-08-28 Thread Volker Simonis
On Fri, 25 Aug 2023 22:22:43 GMT, Mandy Chung wrote: >> 8268829: Provide an optimized way to walk the stack with Class object only >> >> `StackWalker::walk` creates one `StackFrame` per frame and the current >> implementation >> allocates one `StackFrameInfo` and one `MemberName` objects per

Re: RFR: 8313765: Invalid CEN header (invalid zip64 extra data field size) [v9]

2023-08-15 Thread Volker Simonis
On Tue, 15 Aug 2023 18:43:36 GMT, Lance Andersen wrote: >> This PR updates the extra field validation added as part of >> [JDK-8302483](https://bugs.openjdk.org/browse/JDK-8302483) to deal with >> issues seen with 3rd party tools/libraries where a ZipException may be >> encountered when

Re: RFR: 8313765: Invalid CEN header (invalid zip64 extra data field size) [v6]

2023-08-15 Thread Volker Simonis
On Tue, 15 Aug 2023 21:05:16 GMT, Sergey Bylokhov wrote: > I have provided a test.zip file above which passed the zip integrity test via > "zip -T" and can be unzip w/o errors, but rejected by the openjdk. That zip > was created based on the actual specification, and not on the wiki. Did you

Re: RFR: 8313765: Invalid CEN header (invalid zip64 extra data field size) [v9]

2023-08-15 Thread Volker Simonis
On Tue, 15 Aug 2023 18:43:36 GMT, Lance Andersen wrote: >> This PR updates the extra field validation added as part of >> [JDK-8302483](https://bugs.openjdk.org/browse/JDK-8302483) to deal with >> issues seen with 3rd party tools/libraries where a ZipException may be >> encountered when

Re: RFR: 8313765: Invalid CEN header (invalid zip64 extra data field size) [v6]

2023-08-15 Thread Volker Simonis
On Tue, 15 Aug 2023 16:01:37 GMT, Sergey Bylokhov wrote: > Other than that there are no limitation on the size of extended block, it > could be 0, 20, 100 , etc. But it should contain correct data if necessary > and should not be larger than the surrounding "chunk". This seems to be a very

Re: RFR: 8313765: Invalid CEN header (invalid zip64 extra data field size) [v8]

2023-08-15 Thread Volker Simonis
On Tue, 15 Aug 2023 16:41:35 GMT, Lance Andersen wrote: >> This PR updates the extra field validation added as part of >> [JDK-8302483](https://bugs.openjdk.org/browse/JDK-8302483) to deal with >> issues seen with 3rd party tools/libraries where a ZipException may be >> encountered when

Re: RFR: 8313765: Invalid CEN header (invalid zip64 extra data field size) [v2]

2023-08-14 Thread Volker Simonis
On Mon, 14 Aug 2023 17:44:06 GMT, Lance Andersen wrote: >> src/java.base/share/classes/java/util/zip/ZipFile.java line 1364: >> >>> 1362: * As the fields must appear in order, the block size >>> indicates which >>> 1363: * fields to expect: >>> 1364: * 0

Re: RFR: 8313765: Invalid CEN header (invalid zip64 extra data field size) [v5]

2023-08-14 Thread Volker Simonis
On Mon, 14 Aug 2023 21:08:54 GMT, Lance Andersen wrote: >> This PR updates the extra field validation added as part of >> [JDK-8302483](https://bugs.openjdk.org/browse/JDK-8302483) to deal with >> issues seen with 3rd party tools/libraries where a ZipException may be >> encountered when

Re: RFR: 8313765: Invalid CEN header (invalid zip64 extra data field size) [v3]

2023-08-14 Thread Volker Simonis
On Mon, 14 Aug 2023 18:49:25 GMT, Lance Andersen wrote: >> This PR updates the extra field validation added as part of >> [JDK-8302483](https://bugs.openjdk.org/browse/JDK-8302483) to deal with >> issues seen with 3rd party tools/libraries where a ZipException may be >> encountered when

Re: RFR: 8313765: Invalid CEN header (invalid zip64 extra data field size) [v2]

2023-08-14 Thread Volker Simonis
On Mon, 14 Aug 2023 17:33:30 GMT, Lance Andersen wrote: >> This PR updates the extra field validation added as part of >> [JDK-8302483](https://bugs.openjdk.org/browse/JDK-8302483) to deal with >> issues seen with 3rd party tools/libraries where a ZipException may be >> encountered when

Re: RFR: 8313765: Invalid CEN header (invalid zip64 extra data field size) [v2]

2023-08-14 Thread Volker Simonis
On Mon, 14 Aug 2023 17:33:30 GMT, Lance Andersen wrote: >> This PR updates the extra field validation added as part of >> [JDK-8302483](https://bugs.openjdk.org/browse/JDK-8302483) to deal with >> issues seen with 3rd party tools/libraries where a ZipException may be >> encountered when

Re: RFR: 8311500: StackWalker.getCallerClass() throws UOE if invoked reflectively [v3]

2023-07-17 Thread Volker Simonis
On Tue, 11 Jul 2023 15:47:35 GMT, Volker Simonis wrote: >> As the included jtreg test demonstrates, `StackWalker.getCallerClass()` can >> throw an `UnsupportedOperationException` if called reflectively. Currently >> this only happens if we invoke `StackWalker.getCallerCl

Re: RFR: 8311500: StackWalker.getCallerClass() throws UOE if invoked reflectively [v3]

2023-07-14 Thread Volker Simonis
On Thu, 13 Jul 2023 19:27:01 GMT, Mandy Chung wrote: > I wonder if we should move the check to throw UOE if called by > caller-sensitive method in Java as a general guidance to implement the > runtime in Java if desirable. That means it requires the VM to fill not only > the class in the

Re: RFR: 8311500: StackWalker.getCallerClass() throws UOE if invoked reflectively [v3]

2023-07-14 Thread Volker Simonis
On Fri, 14 Jul 2023 08:39:44 GMT, ExE Boss wrote: >> Volker Simonis has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Fixed case when calling getCallerClass() from a @CallerSensitive method >> reflective

Re: RFR: 8311500: StackWalker.getCallerClass() throws UOE if invoked reflectively [v2]

2023-07-11 Thread Volker Simonis
On Thu, 6 Jul 2023 07:16:19 GMT, David Holmes wrote: >> Volker Simonis has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Rename new parameter according to the HS coding conventions > > src/hotspot/shar

Re: RFR: 8311500: StackWalker.getCallerClass() can throw if invoked reflectively [v3]

2023-07-11 Thread Volker Simonis
hNextBatch()` which calls > `StackWalk::fill_in_frames()` (the same method that already fetched the > initial batch of frames). > > Following is a stacktrace of what I've explained so far: > > Native frames: (J=compiled Java code, j=interpreted, Vv=VM code, C=native >

Re: RFR: 8311500: StackWalker.getCallerClass() can throw if invoked reflectively [v2]

2023-07-11 Thread Volker Simonis
On Wed, 5 Jul 2023 17:25:24 GMT, Volker Simonis wrote: >> As the included jtreg test demonstrates, `StackWalker.getCallerClass()` can >> throw an `UnsupportedOperationException` if called reflectively. Currently >> this only happens if we invoke `StackWalker.getCallerCl

Re: RFR: 8311645: Memory leak in jspawnhelper spawnChild after JDK-8307990

2023-07-10 Thread Volker Simonis
On Mon, 10 Jul 2023 18:38:11 GMT, Aleksey Shipilev wrote: > Hold on a sec, let's make sure the GitHub Actions complete with all green. We > can techinically wait after `/integrate` is said for sponsored changeset, but > it is cleaner process-wise to wait for GHA all-clear/all-green. Sure

Re: RFR: 8311645: Memory leak in jspawnhelper spawnChild after JDK-8307990

2023-07-10 Thread Volker Simonis
On Sat, 8 Jul 2023 02:05:27 GMT, Jenny Shivayogi wrote: > Free-ing 'buf' before two conditional return statements introduced by > JDK-8307990 Thanks, looks good now. I can sponsor your change once you `/integrate`. - Marked as reviewed by simonis (Reviewer). PR Review:

Re: RFR: 8311645: Memory leak in jspawnhelper spawnChild after JDK-8307990

2023-07-10 Thread Volker Simonis
On Sat, 8 Jul 2023 02:05:27 GMT, Jenny Shivayogi wrote: > Free-ing 'buf' before two conditional return statements introduced by > JDK-8307990 Hi @kspeeyu , Thanks for fixing this issue. The fix looks good except the whitespace issue already mentioned before. Notice that we don't use TABs

Re: RFR: 8311500: StackWalker.getCallerClass() can throw if invoked reflectively [v2]

2023-07-07 Thread Volker Simonis
On Wed, 5 Jul 2023 19:14:07 GMT, Mandy Chung wrote: >> Volker Simonis has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Rename new parameter according to the HS coding conventions > > Thanks for c

Re: RFR: 8311500: StackWalker.getCallerClass() can throw if invoked reflectively [v2]

2023-07-06 Thread Volker Simonis
On Wed, 5 Jul 2023 19:14:07 GMT, Mandy Chung wrote: > Thanks for catching this issue. I agree that `Method::invoke` should be > skipped the caller-sensitive test in this case but the fix isn't quite right. > The caller-sensitive test should apply in any batch. For example, `CSM` calls >

Re: RFR: 8311500: StackWalker.getCallerClass() can throw if invoked reflectively [v2]

2023-07-05 Thread Volker Simonis
On Wed, 5 Jul 2023 14:41:21 GMT, Aleksey Shipilev wrote: > Neither PR nor the bug describes what the root cause it, so it is very hard > to see if patch makes sense. Please describe the problem and solution briefly? Fair enough :) I've put the details into the initial comment and also renamed

Re: RFR: 8311500: StackWalker.getCallerClass() can throw if invoked reflectively [v2]

2023-07-05 Thread Volker Simonis
hNextBatch()` which calls > `StackWalk::fill_in_frames()` (the same method that already fetched the > initial batch of frames). > > Following is a stacktrace of what I've explained so far: > > Native frames: (J=compiled Java code, j=interpreted, Vv=VM code, C=native >

RFR: 8311500: StackWalker.getCallerClass() can throw if invoked reflectively

2023-07-05 Thread Volker Simonis
As the included jtreg test demonstrates, `StackWalker.getCallerClass()` can throw an `UnsupportedOperationException` if called reflectively. Currently this only happens if we invoke `StackWalker.getCallerClass()` recursively reflectively, but this issue will become more prominent once we fix

Re: RFR: JDK-8310265: (process) jspawnhelper should not use argv[0] [v3]

2023-06-20 Thread Volker Simonis
On Tue, 20 Jun 2023 16:48:42 GMT, Thomas Stuefe wrote: >> Reported by [jarabe...@gmail.com](mailto:jarabe...@gmail.com) [1] >> >> jspawnhelper uses argv[0] to receive the fd string from the parent. That >> breaks with conventions and trips over certain tools like binfmt_misc. >> >> For

Re: RFR: JDK-8310265: (process) jspawnhelper should not use argv[0] [v2]

2023-06-19 Thread Volker Simonis
On Mon, 19 Jun 2023 06:07:26 GMT, Thomas Stuefe wrote: >> Reported by [jarabe...@gmail.com](mailto:jarabe...@gmail.com) [1] >> >> jspawnhelper uses argv[0] to receive the fd string from the parent. That >> breaks with conventions and trips over certain tools like binfmt_misc. >> >> For

Re: RFR: 8284493: Improve computeNextExponential tail performance and accuracy

2023-06-06 Thread Volker Simonis
On Wed, 6 Apr 2022 17:47:53 GMT, Chris Hennick wrote: > This PR improves both the worst-case performance of `nextExponential` and > `nextGaussian` and the distribution of output at the tails. It fixes the > following imperfections: > > * Repeatedly adding DoubleZigguratTables.exponentialX0 to

Integrated: 8307990: jspawnhelper must close its writing side of a pipe before reading from it

2023-06-01 Thread Volker Simonis
On Fri, 12 May 2023 15:24:19 GMT, Volker Simonis wrote: > Since JDK13, executing commands in a sub-process defaults to the so called > `POSIX_SPAWN` launching mechanism (i.e. > `-Djdk.lang.Process.launchMechanism=POSIX_SPAWN`) on Linux. This works by > using `posix_spawn(3)` to f

Re: RFR: 8307990: jspawnhelper must close its writing side of a pipe before reading from it

2023-06-01 Thread Volker Simonis
On Fri, 19 May 2023 15:43:30 GMT, Roger Riggs wrote: >>> Sorry, but I don't understand this argument. If we do a short read we will >>> work with corrupted ChildStuff and SpawnInfo >>> structures. This can in the extreme case execute arbitrary code (e.g. if >>> ChildStuff.argv is not fully

Re: RFR: 8307990: jspawnhelper must close its writing side of a pipe before reading from it [v7]

2023-05-30 Thread Volker Simonis
On Fri, 26 May 2023 17:23:01 GMT, Thomas Stuefe wrote: > Still think it would be cleaner and simpler to set the FD in the parent to > CLOEXEC, before doing posix_spawn, and at the same time set the childStuff > variable to -1 to prevent the child from attempting to re-close it. > Reconsider?

Re: RFR: 8307990: jspawnhelper must close its writing side of a pipe before reading from it [v7]

2023-05-30 Thread Volker Simonis
On Thu, 25 May 2023 15:25:40 GMT, Volker Simonis wrote: >> Since JDK13, executing commands in a sub-process defaults to the so called >> `POSIX_SPAWN` launching mechanism (i.e. >> `-Djdk.lang.Process.launchMechanism=POSIX_SPAWN`) on Linux. This works by >> using `po

Re: RFR: 8307990: jspawnhelper must close its writing side of a pipe before reading from it [v8]

2023-05-30 Thread Volker Simonis
did some cleanup: > - in `jspawnhelper.c` correctly chek the result of `readFully()` > - in `ProcessImpl_md.c` use `restartableWrite()` instead of `write()` to > write the command data from the parent process to `jspawnhelper` > - in `childproc.{c,h}` remove remaining traces of `MODE

Re: RFR: 8307990: jspawnhelper must close its writing side of a pipe before reading from it [v7]

2023-05-25 Thread Volker Simonis
On Thu, 25 May 2023 15:25:40 GMT, Volker Simonis wrote: >> Since JDK13, executing commands in a sub-process defaults to the so called >> `POSIX_SPAWN` launching mechanism (i.e. >> `-Djdk.lang.Process.launchMechanism=POSIX_SPAWN`) on Linux. This works by >> using `po

Re: RFR: 8307990: jspawnhelper must close its writing side of a pipe before reading from it [v7]

2023-05-25 Thread Volker Simonis
did some cleanup: > - in `jspawnhelper.c` correctly chek the result of `readFully()` > - in `ProcessImpl_md.c` use `restartableWrite()` instead of `write()` to > write the command data from the parent process to `jspawnhelper` > - in `childproc.{c,h}` remove remaining traces of `MODE

Re: RFR: 8307990: jspawnhelper must close its writing side of a pipe before reading from it

2023-05-24 Thread Volker Simonis
On Fri, 19 May 2023 15:43:30 GMT, Roger Riggs wrote: >>> Sorry, but I don't understand this argument. If we do a short read we will >>> work with corrupted ChildStuff and SpawnInfo >>> structures. This can in the extreme case execute arbitrary code (e.g. if >>> ChildStuff.argv is not fully

Re: RFR: 8307990: jspawnhelper must close its writing side of a pipe before reading from it [v6]

2023-05-24 Thread Volker Simonis
did some cleanup: > - in `jspawnhelper.c` correctly chek the result of `readFully()` > - in `ProcessImpl_md.c` use `restartableWrite()` instead of `write()` to > write the command data from the parent process to `jspawnhelper` > - in `childproc.{c,h}` remove remaining traces of `MODE

Re: RFR: 8307990: jspawnhelper must close its writing side of a pipe before reading from it [v4]

2023-05-23 Thread Volker Simonis
On Tue, 23 May 2023 05:34:15 GMT, Thomas Stuefe wrote: >> Volker Simonis has updated the pull request with a new target base due to a >> merge or a rebase. The pull request now contains five commits: >> >> - Merge branch 'master' into JDK-8307990 >> -

Re: RFR: 8307990: jspawnhelper must close its writing side of a pipe before reading from it [v5]

2023-05-23 Thread Volker Simonis
did some cleanup: > - in `jspawnhelper.c` correctly chek the result of `readFully()` > - in `ProcessImpl_md.c` use `restartableWrite()` instead of `write()` to > write the command data from the parent process to `jspawnhelper` > - in `childproc.{c,h}` remove remaining traces of `MODE

Re: RFR: 8307990: jspawnhelper must close its writing side of a pipe before reading from it

2023-05-23 Thread Volker Simonis
On Fri, 19 May 2023 15:43:30 GMT, Roger Riggs wrote: >>> Sorry, but I don't understand this argument. If we do a short read we will >>> work with corrupted ChildStuff and SpawnInfo >>> structures. This can in the extreme case execute arbitrary code (e.g. if >>> ChildStuff.argv is not fully

Re: RFR: 8307990: jspawnhelper must close its writing side of a pipe before reading from it [v4]

2023-05-23 Thread Volker Simonis
On Mon, 22 May 2023 10:22:16 GMT, Volker Simonis wrote: >> Since JDK13, executing commands in a sub-process defaults to the so called >> `POSIX_SPAWN` launching mechanism (i.e. >> `-Djdk.lang.Process.launchMechanism=POSIX_SPAWN`) on Linux. This works by >> using `po

Re: RFR: 8307990: jspawnhelper must close its writing side of a pipe before reading from it

2023-05-22 Thread Volker Simonis
On Wed, 17 May 2023 20:03:37 GMT, Joe Darcy wrote: > > > Should this issue have a CSR for any behavioral changes? > > > > > > Well, you can certainly argue that every bug fix is a behavioral changes, > > right :) > > But seriously, I don't see how this PR could require a CSR. The only > >

Re: RFR: 8307990: jspawnhelper must close its writing side of a pipe before reading from it [v4]

2023-05-22 Thread Volker Simonis
did some cleanup: > - in `jspawnhelper.c` correctly chek the result of `readFully()` > - in `ProcessImpl_md.c` use `restartableWrite()` instead of `write()` to > write the command data from the parent process to `jspawnhelper` > - in `childproc.{c,h}` remove remaining traces of `MODE

Re: RFR: 8307990: jspawnhelper must close its writing side of a pipe before reading from it [v3]

2023-05-22 Thread Volker Simonis
On Thu, 18 May 2023 05:58:26 GMT, Thomas Stuefe wrote: >> Volker Simonis has updated the pull request incrementally with one >> additional commit since the last revision: >> >> REALLY adding the test :) > > src/java.base/unix/native/libjava/childproc.c line

Re: RFR: 8307990: jspawnhelper must close its writing side of a pipe before reading from it [v3]

2023-05-22 Thread Volker Simonis
On Wed, 17 May 2023 17:07:13 GMT, Roger Riggs wrote: >> Volker Simonis has updated the pull request incrementally with one >> additional commit since the last revision: >> >> REALLY adding the test :) > > test/jdk/java/lang/ProcessBuilder/JspawnhelperPro

Re: RFR: 8307990: jspawnhelper must close its writing side of a pipe before reading from it [v3]

2023-05-22 Thread Volker Simonis
On Wed, 17 May 2023 17:05:54 GMT, Roger Riggs wrote: >> Volker Simonis has updated the pull request incrementally with one >> additional commit since the last revision: >> >> REALLY adding the test :) > > test/jdk/java/lang/ProcessBuilder/JspawnhelperP

Re: RFR: 8307990: jspawnhelper must close its writing side of a pipe before reading from it [v3]

2023-05-22 Thread Volker Simonis
On Wed, 17 May 2023 17:01:53 GMT, Roger Riggs wrote: >> Volker Simonis has updated the pull request incrementally with one >> additional commit since the last revision: >> >> REALLY adding the test :) > > test/jdk/java/lang/ProcessBuilder/JspawnhelperP

Re: RFR: 8307990: jspawnhelper must close its writing side of a pipe before reading from it [v3]

2023-05-22 Thread Volker Simonis
On Wed, 17 May 2023 17:08:34 GMT, Roger Riggs wrote: >> Volker Simonis has updated the pull request incrementally with one >> additional commit since the last revision: >> >> REALLY adding the test :) > > test/jdk/java/lang/ProcessBuilder/JspawnhelperPro

Re: RFR: 8307990: jspawnhelper must close its writing side of a pipe before reading from it

2023-05-17 Thread Volker Simonis
On Wed, 17 May 2023 14:42:45 GMT, Roger Riggs wrote: > if parent process dies prematurely @RogerRiggs , I've created a release note for the issue based on your suggestions. Please feel free to proof-read and enhance it :) - PR Comment:

Re: RFR: 8307990: jspawnhelper must close its writing side of a pipe before reading from it [v2]

2023-05-17 Thread Volker Simonis
On Wed, 17 May 2023 14:10:59 GMT, Thomas Stuefe wrote: >> Volker Simonis has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Added test case > > src/java.base/unix/native/libjava/ProcessImpl_md.c line 490:

Re: RFR: 8307990: jspawnhelper must close its writing side of a pipe before reading from it [v2]

2023-05-17 Thread Volker Simonis
On Wed, 17 May 2023 13:46:29 GMT, Thomas Stuefe wrote: >> Volker Simonis has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Added test case > > src/java.base/unix/native/jspawnhelper/jspawnhelper.c line 140:

Re: RFR: 8307990: jspawnhelper must close its writing side of a pipe before reading from it [v3]

2023-05-17 Thread Volker Simonis
did some cleanup: > - in `jspawnhelper.c` correctly chek the result of `readFully()` > - in `ProcessImpl_md.c` use `restartableWrite()` instead of `write()` to > write the command data from the parent process to `jspawnhelper` > - in `childproc.{c,h}` remove remaining traces of `MODE

Re: RFR: 8307990: jspawnhelper must close its writing side of a pipe before reading from it [v2]

2023-05-17 Thread Volker Simonis
On Wed, 17 May 2023 14:55:00 GMT, Roger Riggs wrote: >> Volker Simonis has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Added test case > > src/java.base/unix/native/libjava/childproc.c line 408: >

Re: RFR: 8307990: jspawnhelper must close its writing side of a pipe before reading from it

2023-05-17 Thread Volker Simonis
On Mon, 15 May 2023 19:55:45 GMT, Bernd wrote: > Independent of the actual fix, it was mentioned that it can block listening > sockets, aren’t those close on exec? Also should a write and read timeout be > used in addition? At least to call the close descriptor code before retrying? While the

Re: RFR: 8307990: jspawnhelper must close its writing side of a pipe before reading from it

2023-05-17 Thread Volker Simonis
On Mon, 15 May 2023 20:00:28 GMT, Joe Darcy wrote: > Should this issue have a CSR for any behavioral changes? Well, you can certainly argue that every bug fix is a behavioral changes, right :) But seriously, I don't see how this PR could require a CSR. The only behavioral change is really

Re: RFR: 8307990: jspawnhelper must close its writing side of a pipe before reading from it

2023-05-17 Thread Volker Simonis
On Mon, 15 May 2023 18:45:24 GMT, Roger Riggs wrote: >> Since JDK13, executing commands in a sub-process defaults to the so called >> `POSIX_SPAWN` launching mechanism (i.e. >> `-Djdk.lang.Process.launchMechanism=POSIX_SPAWN`) on Linux. This works by >> using `posix_spawn(3)` to firstly start

Re: RFR: 8307990: jspawnhelper must close its writing side of a pipe before reading from it

2023-05-17 Thread Volker Simonis
On Tue, 16 May 2023 12:32:44 GMT, Thomas Stuefe wrote: > > > > I wonder if @Martin-Buchholz is able to look at this one? > > > > My concern with changes like this is that they fix an issue but then > > > > have unexpected side-effects themselves. > > > > > > > > > Are there any specific

Re: RFR: 8307990: jspawnhelper must close its writing side of a pipe before reading from it [v2]

2023-05-17 Thread Volker Simonis
did some cleanup: > - in `jspawnhelper.c` correctly chek the result of `readFully()` > - in `ProcessImpl_md.c` use `restartableWrite()` instead of `write()` to > write the command data from the parent process to `jspawnhelper` > - in `childproc.{c,h}` remove remaining traces of `MODE

Re: RFR: 8307990: jspawnhelper must close its writing side of a pipe before reading from it

2023-05-15 Thread Volker Simonis
On Mon, 15 May 2023 16:31:38 GMT, Thomas Stuefe wrote: > The fact that this error is in there since JDK 13 is scary. The error is there since ten years since the `posix_spawn` mechanism was initially introduced with [JDK-5049299](https://bugs.openjdk.org/browse/JDK-5049299) for JDK 8 and

Re: RFR: 8307990: jspawnhelper must close its writing side of a pipe before reading from it

2023-05-15 Thread Volker Simonis
On Fri, 12 May 2023 15:24:19 GMT, Volker Simonis wrote: > Since JDK13, executing commands in a sub-process defaults to the so called > `POSIX_SPAWN` launching mechanism (i.e. > `-Djdk.lang.Process.launchMechanism=POSIX_SPAWN`) on Linux. This works by > using `posix_spawn(3)` to f

Re: RFR: 8307990: jspawnhelper must close its writing side of a pipe before reading from it

2023-05-15 Thread Volker Simonis
On Fri, 12 May 2023 15:24:19 GMT, Volker Simonis wrote: > Since JDK13, executing commands in a sub-process defaults to the so called > `POSIX_SPAWN` launching mechanism (i.e. > `-Djdk.lang.Process.launchMechanism=POSIX_SPAWN`) on Linux. This works by > using `posix_spawn(3)` to f

Re: RFR: 8307403: java/util/zip/DeInflate.java timed out

2023-05-15 Thread Volker Simonis
On Fri, 12 May 2023 12:47:28 GMT, Jaikiran Pai wrote: > Can I please get a review of this test only change which addresses the issue > noted in https://bugs.openjdk.org/browse/JDK-8307403? > > When we recently did a change in https://bugs.openjdk.org/browse/JDK-8299748, > there was a

Re: RFR: 8307990: jspawnhelper must close its writing side of a pipe before reading from it

2023-05-15 Thread Volker Simonis
On Sun, 14 May 2023 17:28:33 GMT, Thomas Stuefe wrote: > Trying to understand this, and the scope of the problem: > > * Parent process opens "in" pipe. > > * Now owns read and write end of pipe. > > * Parent forks jspawnhelper. > > * jspawnhelper inherits handles. Now there

Re: RFR: 8307990: jspawnhelper must close its writing side of a pipe before reading from it

2023-05-15 Thread Volker Simonis
On Fri, 12 May 2023 21:37:36 GMT, Roger Riggs wrote: > Looks ok. > > Is it practical to write a test for this situation? Can I assume you've > validated the improvement as a remedy for the observed hangs? I thought about a test but couldn't come up with a practical way to write it. The JVM

RFR: 8307990: jspawnhelper must close its writing side of a pipe before reading from it

2023-05-12 Thread Volker Simonis
Since JDK13, executing commands in a sub-process defaults to the so called `POSIX_SPAWN` launching mechanism (i.e. `-Djdk.lang.Process.launchMechanism=POSIX_SPAWN`) on Linux. This works by using `posix_spawn(3)` to firstly start a tiny helper program called `jspawnhelper` in a subprocess. In a

Integrated: 8306461: ObjectInputStream::readObject() should handle negative array sizes without throwing NegativeArraySizeExceptions

2023-05-08 Thread Volker Simonis
On Wed, 19 Apr 2023 16:47:33 GMT, Volker Simonis wrote: > This issue was reported by: Yakov Shafranovich > ([yako...@amazon.com](mailto:yako...@amazon.com)) > > Currently, `ObjectInputStream::readObject()` doesn't explicitly checks for a > negative array length in the deseria

Re: RFR: 8306461: ObjectInputStream::readObject() should handle negative array sizes without throwing NegativeArraySizeExceptions [v6]

2023-05-08 Thread Volker Simonis
On Tue, 2 May 2023 18:02:41 GMT, Volker Simonis wrote: >> This issue was reported by: Yakov Shafranovich >> ([yako...@amazon.com](mailto:yako...@amazon.com)) >> >> Currently, `ObjectInputStream::readObject()` doesn't explicitly checks for a >> negative array

Re: RFR: 8306461: ObjectInputStream::readObject() should handle negative array sizes without throwing NegativeArraySizeExceptions [v4]

2023-05-02 Thread Volker Simonis
On Tue, 2 May 2023 16:59:00 GMT, Aleksey Shipilev wrote: >> Volker Simonis has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Throw StreamCorruptedException instead of InvalidClassException and handle >> neg

Re: RFR: 8306461: ObjectInputStream::readObject() should handle negative array sizes without throwing NegativeArraySizeExceptions [v6]

2023-05-02 Thread Volker Simonis
ctInputFilter.FilterInfo::arrayLength()` which is defined as: > > Returns: > the non-negative number of array elements when deserializing an array of the > class, otherwise -1 > > but currently returns a negative value if the array length is negative. Volker Simonis has update

Re: RFR: 8306461: ObjectInputStream::readObject() should handle negative array sizes without throwing NegativeArraySizeExceptions [v4]

2023-05-02 Thread Volker Simonis
On Tue, 2 May 2023 16:06:50 GMT, Aleksey Shipilev wrote: >> Volker Simonis has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Throw StreamCorruptedException instead of InvalidClassException and handle >> neg

Re: RFR: 8306461: ObjectInputStream::readObject() should handle negative array sizes without throwing NegativeArraySizeExceptions [v5]

2023-05-02 Thread Volker Simonis
ctInputFilter.FilterInfo::arrayLength()` which is defined as: > > Returns: > the non-negative number of array elements when deserializing an array of the > class, otherwise -1 > > but currently returns a negative value if the array length is negative. Volker Simonis has update

Re: RFR: 8306461: ObjectInputStream::readObject() should handle negative array sizes without throwing NegativeArraySizeExceptions [v4]

2023-05-02 Thread Volker Simonis
On Tue, 2 May 2023 15:52:03 GMT, Roger Riggs wrote: >> Volker Simonis has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Throw StreamCorruptedException instead of InvalidClassException and handle >> neg

Re: RFR: 8306461: ObjectInputStream::readObject() should handle negative array sizes without throwing NegativeArraySizeExceptions [v3]

2023-05-02 Thread Volker Simonis
On Thu, 27 Apr 2023 21:24:04 GMT, Roger Riggs wrote: >> Volker Simonis 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 contai

Re: RFR: 8306461: ObjectInputStream::readObject() should handle negative array sizes without throwing NegativeArraySizeExceptions [v4]

2023-05-02 Thread Volker Simonis
ctInputFilter.FilterInfo::arrayLength()` which is defined as: > > Returns: > the non-negative number of array elements when deserializing an array of the > class, otherwise -1 > > but currently returns a negative value if the array length is negative. Volker Simonis has update

Re: RFR: 8299748: java/util/zip/Deinflate.java failing on s390x [v10]

2023-04-28 Thread Volker Simonis
On Fri, 28 Apr 2023 11:18:56 GMT, Amit Kumar wrote: >> DeInflate.java test fails on s390x platform because size for out1 array >> which is responsible for storing the compressed data is insufficient. And >> being unable to write whole compressed data on array, on s390 whole data >> can't be

Re: RFR: 8299748: java/util/zip/Deinflate.java failing on s390x [v7]

2023-04-28 Thread Volker Simonis
On Fri, 28 Apr 2023 09:26:52 GMT, Volker Simonis wrote: >> Amit Kumar has updated the pull request incrementally with one additional >> commit since the last revision: >> >> suggestion from @simonis > > test/jdk/java/util/zip/DeInflate.java line 16

Re: RFR: 8299748: java/util/zip/Deinflate.java failing on s390x [v7]

2023-04-28 Thread Volker Simonis
On Fri, 28 Apr 2023 09:23:59 GMT, Amit Kumar wrote: >> DeInflate.java test fails on s390x platform because size for out1 array >> which is responsible for storing the compressed data is insufficient. And >> being unable to write whole compressed data on array, on s390 whole data >> can't be

Re: RFR: 8299748: java/util/zip/Deinflate.java failing on s390x [v7]

2023-04-28 Thread Volker Simonis
On Fri, 28 Apr 2023 09:08:18 GMT, Amit Kumar wrote: >> DeInflate.java test fails on s390x platform because size for out1 array >> which is responsible for storing the compressed data is insufficient. And >> being unable to write whole compressed data on array, on s390 whole data >> can't be

Re: RFR: 8299748: java/util/zip/Deinflate.java failing on s390x [v6]

2023-04-28 Thread Volker Simonis
On Thu, 27 Apr 2023 17:05:24 GMT, Amit Kumar wrote: >> DeInflate.java test fails on s390x platform because size for out1 array >> which is responsible for storing the compressed data is insufficient. And >> being unable to write whole compressed data on array, on s390 whole data >> can't be

Re: RFR: 8306461: ObjectInputStream::readObject() should handle negative array sizes without throwing NegativeArraySizeExceptions [v3]

2023-04-27 Thread Volker Simonis
On Wed, 26 Apr 2023 12:47:47 GMT, Roger Riggs wrote: >> Volker Simonis 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 contai

  1   2   >