Re: RFR: 8236569: -Xss not multiple of 4K does not work for the main thread on macOS [v5]

2021-06-08 Thread Thomas Stuefe
On Tue, 8 Jun 2021 16:45:12 GMT, Henry Jen wrote: >> src/java.base/unix/native/libjli/java_md.c line 666: >> >>> 664: return page_size * pages; >>> 665: } >>> 666: } >> >> Could probably be shortened to something like this: >> >> >> size_t pagesize =

Re: RFR: 8236569: -Xss not multiple of 4K does not work for the main thread on macOS [v5]

2021-06-08 Thread Thomas Stuefe
On Mon, 7 Jun 2021 03:18:32 GMT, Henry Jen wrote: >> …d on macOS >> >> This patch simply round up the specified stack size to multiple of the >> system page size. >> >> Test is trivial, simply run java with -Xss option against following code. On >> MacOS, before the fix, running with

Re: RFR: 8265518: C1: Intrinsic support for Preconditions.checkIndex [v13]

2021-06-12 Thread Thomas Stuefe
On Sat, 12 Jun 2021 06:29:50 GMT, Volker Simonis wrote: > This change removed a product flag so I wonder how it could be integrated > without a CSR? And if the intention was to remove it, should it not have been marked as obsolete first? - PR:

Re: RFR: 8265518: C1: Intrinsic support for Preconditions.checkIndex [v13]

2021-06-12 Thread Thomas Stuefe
On Wed, 9 Jun 2021 08:53:40 GMT, Yi Yang wrote: >> The JDK codebase re-created many variants of checkIndex(`grep -I -r >> 'cehckIndex' jdk/`). A notable variant is java.nio.Buffer.checkIndex, which >> annotated with @IntrinsicCandidate and it only has a corresponding C1 >> intrinsic version.

Re: RFR: 8236569: -Xss not multiple of 4K does not work for the main thread on macOS [v5]

2021-06-08 Thread Thomas Stuefe
On Mon, 7 Jun 2021 03:18:32 GMT, Henry Jen wrote: >> …d on macOS >> >> This patch simply round up the specified stack size to multiple of the >> system page size. >> >> Test is trivial, simply run java with -Xss option against following code. On >> MacOS, before the fix, running with

Re: RFR: 8266168: -Wmaybe-uninitialized happens in check_code.c

2021-05-06 Thread Thomas Stuefe
On Thu, 29 Apr 2021 06:54:33 GMT, Yasumasa Suenaga wrote: > We can see following compiler warning in check_code.c on GCC 11. LGTM. I am not aware of any platform where sizeof(char) is not 1, but good to be prepared :) - Marked as reviewed by stuefe (Reviewer). PR:

Re: [jdk17] RFR: 8269486: CallerAccessTest fails for non server variant

2021-06-28 Thread Thomas Stuefe
On Mon, 28 Jun 2021 13:14:51 GMT, Christoph Göttschkes wrote: > Hi, > > please review this small fix. The test case uses a custom launcher and before > launching the JVM, it adds the "lib" and "lib/server" directories to the > environment variable which controls the native library search

Re: RFR: 8261299: Use-after-free on failure path in LinuxPackage.c, getJvmLauncherLibPath

2021-02-08 Thread Thomas Stuefe
On Mon, 8 Feb 2021 09:05:23 GMT, Aleksey Shipilev wrote: > SonarCloud instance reports a new warning after JDK-8254702: > "Use of memory after it is freed" > > char* getJvmLauncherLibPath(void) { >... > popenStatus = popenCommand(pkgQueryCmd, pkg->name, findLauncherLib, >

Re: RFR: 8261299: Use-after-free on failure path in LinuxPackage.c, getJvmLauncherLibPath

2021-02-08 Thread Thomas Stuefe
On Mon, 8 Feb 2021 09:05:23 GMT, Aleksey Shipilev wrote: > SonarCloud instance reports a new warning after JDK-8254702: > "Use of memory after it is freed" > > char* getJvmLauncherLibPath(void) { >... > popenStatus = popenCommand(pkgQueryCmd, pkg->name, findLauncherLib, >

Re: RFR: 8261154: Memory leak in Java_java_lang_ClassLoader_defineClass0 with long class names [v2]

2021-02-04 Thread Thomas Stuefe
On Thu, 4 Feb 2021 15:53:24 GMT, Claes Redestad wrote: > I'm not sure every platform have always agreed free(NULL) is a noop. I > suspect all the currently supported ones do, though? Its standard behavior. Posix: https://pubs.opengroup.org/onlinepubs/009695399/functions/free.html "If ptr is

Re: RFR: 8261154: Memory leak in Java_java_lang_ClassLoader_defineClass0 with long class names [v2]

2021-02-04 Thread Thomas Stuefe
On Thu, 4 Feb 2021 16:14:57 GMT, Claes Redestad wrote: >> This patch resolves a potential memory leak in >> Java_java_lang_ClassLoader_defineClass0 >> >> I've not figured a good way to write a regression test. A crude way to >> manually verify the leak and the fix is provided by the added

Re: RFR: 8261154: Memory leak in Java_java_lang_ClassLoader_defineClass0 with long class names

2021-02-04 Thread Thomas Stuefe
On Thu, 4 Feb 2021 14:23:56 GMT, Claes Redestad wrote: > This patch resolves a potential memory leak in > Java_java_lang_ClassLoader_defineClass0 > > I've not figured a good way to write a regression test. A crude way to > manually verify the leak and the fix is provided by the added

Re: RFR: 8261449: Micro-optimize JVM_LatestUserDefinedLoader [v2]

2021-02-10 Thread Thomas Stuefe
On Wed, 10 Feb 2021 07:34:59 GMT, Aleksey Shipilev wrote: >> `JVM_LatestUserDefinedLoader` is called normally from >> `ObjectInputStream.resolveClass` -> `VM.latestUserDefinedLoader0`. And it >> takes a measurable time to walk the stack. There is JDK-8173368 that wants >> to replace it with

Re: RFR: 8263509: LdapSchemaParser.readNextTag checks array length incorrectly

2021-03-12 Thread Thomas Stuefe
On Fri, 12 Mar 2021 13:25:31 GMT, Aleksey Shipilev wrote: > SonarCloud rightfully says: > The length of "values" is always ">=0", so update this test to either "==0" > or ">0". > > // make sure at least one value was returned > if(values.length < 0) { // <--- here >

Re: RFR: 8263729: [test] Extend time to wait before destroying child in ProcssBuilder Basic test

2021-03-18 Thread Thomas Stuefe
On Thu, 18 Mar 2021 13:41:01 GMT, Roger Riggs wrote: > The test expects there to be zero output from the child (and it doesn't > matter what state the child is in). > Can the logging from the VM be disabled or re-directed? Not to the extend that it would be guaranteed never to happen. Even if

Re: RFR: 8263729: [test] Extend time to wait before destroying child in ProcssBuilder Basic test

2021-03-17 Thread Thomas Stuefe
On Wed, 17 Mar 2021 17:14:22 GMT, Ioi Lam wrote: >> That complicates the test and the child quite a bit for minimal gain. > > Arbitrary time out has been a reliable source of intermittent failures. > > Since we have spent a lot of time analyzing this failure, I think it's > worthwhile to fix

Re: RFR: 8263729: [test] Extend time to wait before destroying child in ProcssBuilder Basic test [v2]

2021-03-19 Thread Thomas Stuefe
On Fri, 19 Mar 2021 17:01:26 GMT, Roger Riggs wrote: >> test/jdk/java/lang/ProcessBuilder/Basic.java line 2156: >> >>> 2154: case 1: >>> 2155: childArgs.set(1, >>> "-XX:+DisplayVMOutputToStdout"); >>> 2156:

Re: RFR: 8263729: [test] Extend time to wait before destroying child in ProcssBuilder Basic test [v2]

2021-03-19 Thread Thomas Stuefe
On Fri, 19 Mar 2021 14:23:55 GMT, Roger Riggs wrote: >> Intermittent failures on Windows in a test of destroying the child warrant >> extending the time the parent waits after starting the child before >> destroying the child. > > Roger Riggs has updated the pull request incrementally with one

Re: RFR: 8263729: [test] Extend time to wait before destroying child in ProcssBuilder Basic test [v4]

2021-03-19 Thread Thomas Stuefe
On Fri, 19 Mar 2021 18:41:51 GMT, Roger Riggs wrote: >> Intermittent failures on Windows in a test of destroying the child warrant >> extending the time the parent waits after starting the child before >> destroying the child. > > Roger Riggs has updated the pull request incrementally with one

Re: RFR: 8263729: [test] Extend time to wait before destroying child in ProcssBuilder Basic test

2021-03-17 Thread Thomas Stuefe
On Wed, 17 Mar 2021 17:53:14 GMT, Ioi Lam wrote: >> I don't think this is CPU starvation but memory exhaustion. _beginthreadex >> fails with EACCES if it has no resources to start the thread, which in this >> case probably means memory (the other possibility would be >> out-of-HANDLE-space

Re: RFR: 8263729: [test] Extend time to wait before destroying child in ProcssBuilder Basic test

2021-03-17 Thread Thomas Stuefe
On Wed, 17 Mar 2021 19:52:49 GMT, Roger Riggs wrote: > Its a Java Child for consistency across tests and across OS's. > The JavaChild executes a number of specialized commands to consume or provide > data to the parent. > Piecing that together on different OS's would add more variables to the

Re: RFR: 8263729: [test] Extend time to wait before destroying child in ProcssBuilder Basic test

2021-03-17 Thread Thomas Stuefe
On Wed, 17 Mar 2021 19:34:19 GMT, Thomas Stuefe wrote: >> Hmm, maybe the child can create a file to indicate that bootstrap has >> finished. > >> The child does no (zero) writes to either stream. It is invoked only to >> sleep until it is destroyed. >> The

Re: RFR: 8263729: [test] Extend time to wait before destroying child in ProcssBuilder Basic test

2021-03-17 Thread Thomas Stuefe
On Wed, 17 Mar 2021 19:30:54 GMT, Ioi Lam wrote: >> The child does no (zero) writes to either stream. It is invoked only to >> sleep until it is destroyed. >> The purpose of the test is to verify the exception that is thrown when the >> other end(child) of the pipe is closed (because the

Re: RFR: JDK-8256844: Make NMT late-initializable

2021-07-30 Thread Thomas Stuefe
On Fri, 30 Jul 2021 13:03:32 GMT, Zhengyu Gu wrote: > Sorry for late review. > > Did a quick scan and have a few questions, will do more detail reading later. Thanks a lot, I appreciate your feedback! > src/hotspot/share/services/nmtPreInit.hpp line 108: > >> 106: // - lookup speed is

Re: RFR: JDK-8256844: Make NMT late-initializable

2021-07-30 Thread Thomas Stuefe
On Fri, 30 Jul 2021 18:28:44 GMT, Zhengyu Gu wrote: >> So, you are saying that there is no memory that is malloc'd pre-NMT-init >> phase and freed post-NMT-init phase? > > Okay, I see. It just leaks those memory, so the table can be read-only. Exactly. There are a few allocs that either get

Re: RFR: JDK-8256844: Make NMT late-initializable [v2]

2021-08-02 Thread Thomas Stuefe
On Mon, 2 Aug 2021 14:38:23 GMT, Coleen Phillimore wrote: > This looks good. Thanks for fixing the mysterious (to me) cast. Thank you, Coleen! - PR: https://git.openjdk.java.net/jdk/pull/4874

Re: RFR: JDK-8271858: Handle to jimage file inherited into child processes (posix)

2021-08-05 Thread Thomas Stuefe
On Wed, 4 Aug 2021 13:35:59 GMT, Alan Bateman wrote: >> We should not leak the handle to the jimage file (lib/modules) to childs. >> >> JDK-8194734 did solve this for windows. We should use FD_CLOEXEC on Posix as >> well. >> >> Note that this only poses a problem when a child process is

Re: RFR: JDK-8271858: Handle to jimage file inherited into child processes (posix)

2021-08-05 Thread Thomas Stuefe
On Thu, 5 Aug 2021 13:13:27 GMT, Alan Bateman wrote: >> Probably others too, if we care about inheriting read handles to child. >> >> The background is that I am playing with a new test which checks that the VM >> has no open inheritable file descriptors. It is supposed to replace some >>

RFR: JDK-8271858: Handle to jimage file inherited into child processes (posix)

2021-08-05 Thread Thomas Stuefe
We should not leak the handle to the jimage file (lib/modules) to childs. JDK-8194734 did solve this for windows. We should use FD_CLOEXEC on Posix as well. Note that this only poses a problem when a child process is spawned off not via `Runtime.exec` but via another route since the

Re: RFR: JDK-8256844: Make NMT late-initializable [v2]

2021-08-03 Thread Thomas Stuefe
On Sun, 1 Aug 2021 08:17:08 GMT, Thomas Stuefe wrote: >> Short: this patch makes NMT available in custom-launcher scenarios and >> during gtests. It simplifies NMT initialization. It adds a lot of >> NMT-specific testing, cleans them up and makes th

Withdrawn: JDK-8271858: Handle to jimage file inherited into child processes (posix)

2021-08-11 Thread Thomas Stuefe
On Wed, 4 Aug 2021 12:22:24 GMT, Thomas Stuefe wrote: > We should not leak the handle to the jimage file (lib/modules) to childs. > > JDK-8194734 did solve this for windows. We should use FD_CLOEXEC on Posix as > well. > > Note that this only poses a problem when a child

Re: RFR: JDK-8271858: Handle to jimage file inherited into child processes (posix)

2021-08-11 Thread Thomas Stuefe
On Wed, 4 Aug 2021 12:22:24 GMT, Thomas Stuefe wrote: > We should not leak the handle to the jimage file (lib/modules) to childs. > > JDK-8194734 did solve this for windows. We should use FD_CLOEXEC on Posix as > well. > > Note that this only poses a problem when a child

Re: RFR: JDK-8256844: Make NMT late-initializable

2021-07-30 Thread Thomas Stuefe
On Thu, 29 Jul 2021 23:03:46 GMT, Coleen Phillimore wrote: > This is an interesting and it seems a better way to solve this problem. Where > were you all those years ago?? I hope @zhengyu123 has a chance to review it. Thank you! I was here, but we were not yet doing much upstream :) To be

Re: RFR: JDK-8256844: Make NMT late-initializable

2021-07-30 Thread Thomas Stuefe
On Fri, 30 Jul 2021 04:09:32 GMT, Kim Barrett wrote: >> src/hotspot/share/services/nmtPreInit.hpp line 166: >> >>> 164: NMTPreInitAllocation** find_entry(const void* p) const { >>> 165: const unsigned index = index_for_key(p); >>> 166: NMTPreInitAllocation** aa =

Re: RFR: JDK-8256844: Make NMT late-initializable

2021-07-30 Thread Thomas Stuefe
On Fri, 30 Jul 2021 04:03:57 GMT, Kim Barrett wrote: >> src/hotspot/share/services/nmtPreInit.hpp line 128: >> >>> 126: // Returns start of the user data area >>> 127: void* payload() { return this + 1; } >>> 128: const void* payload() const { return this + 1; } >> >> This is

Re: RFR: JDK-8256844: Make NMT late-initializable

2021-07-30 Thread Thomas Stuefe
On Thu, 29 Jul 2021 06:37:36 GMT, David Holmes wrote: > Hi Thomas, > > I had a look through this and it seems reasonable, but I'm not familiar > enough with the details to approve at this stage. > > A few nits below. > > Thanks, > David I did not expect a quick review for this one, so

Re: RFR: JDK-8256844: Make NMT late-initializable [v2]

2021-08-01 Thread Thomas Stuefe
On Fri, 30 Jul 2021 20:14:04 GMT, Zhengyu Gu wrote: >> Thomas Stuefe has updated the pull request incrementally with six additional >> commits since the last revision: >> >> - feedback zhengyu >> - feeback Coleen/Kim (constness fix) >> - Feedback David

Re: RFR: JDK-8256844: Make NMT late-initializable [v2]

2021-08-01 Thread Thomas Stuefe
On Sun, 1 Aug 2021 08:17:08 GMT, Thomas Stuefe wrote: >> Short: this patch makes NMT available in custom-launcher scenarios and >> during gtests. It simplifies NMT initialization. It adds a lot of >> NMT-specific testing, cleans them up and makes th

Re: RFR: JDK-8256844: Make NMT late-initializable [v2]

2021-08-01 Thread Thomas Stuefe
tilites, see all the tests in > test/hotspot/jtreg/gtest.. . It works very well. > - Finally, a new gtest has been written to test the NMT preinit lookup > map in isolation, placed in `gtest/nmt/test_nmtpreinitmap.cpp`. > > - jtreg: > - A new test has been added, `

Re: RFR: JDK-8256844: Make NMT late-initializable

2021-07-31 Thread Thomas Stuefe
On Thu, 29 Jul 2021 06:33:38 GMT, David Holmes wrote: >> Short: this patch makes NMT available in custom-launcher scenarios and >> during gtests. It simplifies NMT initialization. It adds a lot of >> NMT-specific testing, cleans them up and makes them sideeffect-free. >> >> - >> >>

Re: RFR: JDK-8256844: Make NMT late-initializable

2021-07-30 Thread Thomas Stuefe
On Fri, 30 Jul 2021 12:56:59 GMT, Zhengyu Gu wrote: >> Short: this patch makes NMT available in custom-launcher scenarios and >> during gtests. It simplifies NMT initialization. It adds a lot of >> NMT-specific testing, cleans them up and makes them sideeffect-free. >> >> - >> >> NMT

Integrated: JDK-8256844: Make NMT late-initializable

2021-08-04 Thread Thomas Stuefe
On Thu, 22 Jul 2021 14:58:47 GMT, Thomas Stuefe wrote: > Short: this patch makes NMT available in custom-launcher scenarios and during > gtests. It simplifies NMT initialization. It adds a lot of NMT-specific > testing, cleans them up and makes them sideeffect-free. > > ---

Re: RFR: JDK-8256844: Make NMT late-initializable [v2]

2021-08-04 Thread Thomas Stuefe
On Mon, 2 Aug 2021 14:24:54 GMT, Coleen Phillimore wrote: >>> This is an interesting and it seems a better way to solve this problem. >>> Where were you all those years ago?? I hope @zhengyu123 has a chance to >>> review it. >> >> Thank you! I was here, but we were not yet doing much upstream

Re: RFR: 8274293: Build failure on macOS with Xcode 13.0 as vfork is deprecated [v2]

2021-09-25 Thread Thomas Stuefe
On Sat, 25 Sep 2021 01:48:29 GMT, xpbob wrote: >> remove vfork() on Darwin > > xpbob has updated the pull request incrementally with one additional commit > since the last revision: > > Drop drop "Solaris" from the comment The JDK change is fine. AFAICS we have used posix_spawn by default

Re: RFR: 8274293: Build failure on macOS with Xcode 13.0 as vfork is deprecated [v2]

2021-09-25 Thread Thomas Stuefe
On Sat, 25 Sep 2021 01:48:29 GMT, xpbob wrote: >> remove vfork() on Darwin > > xpbob has updated the pull request incrementally with one additional commit > since the last revision: > > Drop drop "Solaris" from the comment .. I opened https://bugs.openjdk.java.net/browse/JDK-8274320 to

Re: RFR: JDK-8256844: Make NMT late-initializable

2021-07-27 Thread Thomas Stuefe
On Mon, 26 Jul 2021 21:08:04 GMT, David Holmes wrote: > Before looking at this, have you checked the startup performance impact? > > Thanks, > David > - Hi David, performance should not be a problem. The potentially costly thing is the underlying hashmap. But we keep it operating with a

RFR: JDK-8256844: Make NMT late-initializable

2021-07-26 Thread Thomas Stuefe
Short: this patch makes NMT available in custom-launcher scenarios and during gtests. It simplifies NMT initialization. It adds a lot of NMT-specific testing. - NMT continues to be an extremely useful tool for SAP to tackle memory problems in the JVM. However, NMT is of limited use

Re: RFR: 8282628: Potential memory leak in sun.font.FontConfigManager.getFontConfig()

2022-03-04 Thread Thomas Stuefe
On Fri, 4 Mar 2022 13:25:12 GMT, Zhengyu Gu wrote: > Please review this small patch that fixes a potential memory leak that > exception return fails to release allocated `cacheDirs` > > Test: > > - [x] jdk_desktop on Linux x86_64 Looks good. - Marked as reviewed by stuefe

Re: RFR: 8253495: CDS generates non-deterministic output [v2]

2022-03-11 Thread Thomas Stuefe
On Fri, 11 Mar 2022 08:28:32 GMT, Ioi Lam wrote: >> Is reproducibility also a topic for users calling -Xdump with custom JNI >> coding? Or maybe having the VM instrumented somehow? Since it seems such an >> easy fix, I would prevent attaching too. At least the user would get a clear >> error

Re: RFR: 8283287: Spring Cleaning for ClassLoader.c

2022-03-17 Thread Thomas Stuefe
On Wed, 16 Mar 2022 21:25:37 GMT, Tyler Steele wrote: > As mentioned in the issue, I'd like to perform the following tidying on > ClassLoader.c > > - Alphabetize includes. > - Replace 'if (ptr == 0)' with 'if (ptr == NULL)'. > - Replace 'return 0' with 'return NULL'. Looks good in general.

Re: RFR: 8283225: [AIX] ClassLoader.c produces incorrect OutOfMemory Exception when length is 0

2022-03-16 Thread Thomas Stuefe
On Tue, 15 Mar 2022 22:58:48 GMT, Tyler Steele wrote: > As described in the linked issue, NullClassBytesTest fails due an > OutOfMemoryError produced on AIX when the test calls defineClass with a byte > array of size of 0. The native implementation of defineClass then calls > malloc with a

Re: RFR: 8283225: [AIX] ClassLoader.c produces incorrect OutOfMemory Exception when length is 0 [v3]

2022-03-17 Thread Thomas Stuefe
On Thu, 17 Mar 2022 07:44:39 GMT, David Holmes wrote: > Using AIX_ONLY this can be simplified: > > `body = (jbyte *)malloc(length AIX_ONLY( == 0 ? 1 : length));` This is jdk, not hotspot. Do we have AIX_ONLY in the JDK? - PR: https://git.openjdk.java.net/jdk/pull/7829

Re: RFR: 8283225: [AIX] ClassLoader.c produces incorrect OutOfMemory Exception when length is 0 [v3]

2022-03-17 Thread Thomas Stuefe
On Wed, 16 Mar 2022 21:10:14 GMT, Tyler Steele wrote: >> As described in the linked issue, NullClassBytesTest fails due an >> OutOfMemoryError produced on AIX when the test calls defineClass with a byte >> array of size of 0. The native implementation of defineClass then calls >> malloc with

Re: RFR: 8283225: [AIX] ClassLoader.c produces incorrect OutOfMemory Exception when length is 0 [v3]

2022-03-17 Thread Thomas Stuefe
On Wed, 16 Mar 2022 21:10:14 GMT, Tyler Steele wrote: >> As described in the linked issue, NullClassBytesTest fails due an >> OutOfMemoryError produced on AIX when the test calls defineClass with a byte >> array of size of 0. The native implementation of defineClass then calls >> malloc with

Re: RFR: 8253495: CDS generates non-deterministic output

2022-03-10 Thread Thomas Stuefe
On Fri, 11 Mar 2022 07:03:00 GMT, David Holmes wrote: > > Well, he does it for `DumpSharedSpaces` only. Are you really worried about > > that one load+conditional jump? > > As I said (and I'm not the only one who says this :) ) "death by a thousand > cuts". Thank you, that is a good

Re: RFR: 8253495: CDS generates non-deterministic output

2022-03-10 Thread Thomas Stuefe
On Fri, 11 Mar 2022 06:50:00 GMT, David Holmes wrote: > I can combine the tests for `MemTracker::tracking_level()` and > `DumpSharedSpaces` into a single test and do more work only when the uncommon > path is taken. This would require some refactoring of the > MemTracker/MallocTracker code.

Re: RFR: 8253495: CDS generates non-deterministic output [v2]

2022-03-10 Thread Thomas Stuefe
On Thu, 10 Mar 2022 19:34:29 GMT, Ioi Lam wrote: >> src/hotspot/share/prims/jvm.cpp line 2887: >> >>> 2885: return; >>> 2886: } >>> 2887: #endif >> >> Should we do this for jni_AttachCurrentThread too? > > This hasn't been necessary for me because jni_AttachCurrentThread is not > called

Re: RFR: 8253495: CDS generates non-deterministic output

2022-03-10 Thread Thomas Stuefe
On Fri, 11 Mar 2022 05:59:00 GMT, David Holmes wrote: > > Thanks for pointing this out. I ran more tests and found that on certain > > platforms, there are other structures that have problems with uninitialized > > gaps. I ended up changing `os::malloc()` to zero the buffer when running > >

Re: RFR: 8253495: CDS generates non-deterministic output [v2]

2022-03-10 Thread Thomas Stuefe
On Wed, 9 Mar 2022 07:58:51 GMT, Thomas Stuefe wrote: >> Ioi Lam has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Fixed zero build > > Hi Ioi, > > some questions, comments inline. > > Like Da

Re: RFR: 8282628: Potential memory leak in sun.font.FontConfigManager.getFontConfig() [v2]

2022-03-08 Thread Thomas Stuefe
On Tue, 8 Mar 2022 15:54:45 GMT, Zhengyu Gu wrote: >> Please review this small patch that fixes a potential memory leak that >> exception return fails to release allocated `cacheDirs` >> >> Test: >> >> - [x] jdk_desktop on Linux x86_64 > > Zhengyu Gu has updated the pull request incrementally

Re: RFR: 8283225: ClassLoader.c produces incorrect OutOfMemory Exception when length is 0 (aix) [v4]

2022-03-18 Thread Thomas Stuefe
On Fri, 18 Mar 2022 02:15:36 GMT, David Holmes wrote: > Update looks good. Sorry for the AIX_ONLY misdirect. > > Thanks, David It would be real nice to have the same set of macros in JDK too, though. - PR: https://git.openjdk.java.net/jdk/pull/7829

Re: RFR: 8253495: CDS generates non-deterministic output [v2]

2022-03-09 Thread Thomas Stuefe
On Wed, 9 Mar 2022 05:10:44 GMT, Ioi Lam wrote: >> This patch makes the result of "java -Xshare:dump" deterministic: >> - Disabled new Java threads from launching. This is harmless. See comments >> in jvm.cpp >> - Fixed a problem in hashtable ordering in heapShared.cpp >> - BasicHashtableEntry

Re: RFR: 8282219: jdk/java/lang/ProcessBuilder/Basic.java fails on AIX [v4]

2022-02-23 Thread Thomas Stuefe
On Wed, 23 Feb 2022 18:49:22 GMT, Ichiroh Takiguchi wrote: >> Run jtreg:jdk/java/lang/ProcessBuilder/Basic.java on AIX. >> The test was failed by: >> Incorrect handling of envstrings containing NULs >> >> According to my investigation, this issue was happened after following >> change was

Re: RFR: 8282219: jdk/java/lang/ProcessBuilder/Basic.java fails on AIX [v4]

2022-02-24 Thread Thomas Stuefe
On Wed, 23 Feb 2022 18:49:22 GMT, Ichiroh Takiguchi wrote: >> Run jtreg:jdk/java/lang/ProcessBuilder/Basic.java on AIX. >> The test was failed by: >> Incorrect handling of envstrings containing NULs >> >> According to my investigation, this issue was happened after following >> change was

Re: RFR: 8282219: jdk/java/lang/ProcessBuilder/Basic.java fails on AIX [v4]

2022-02-24 Thread Thomas Stuefe
On Thu, 24 Feb 2022 18:51:08 GMT, Roger Riggs wrote: >> Ichiroh Takiguchi has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Add null check > > The piece I was missing is that the HotSpot AIX specific code, computes and > sets LIBPATH

Re: RFR: 8278753: Runtime crashes with access violation during JNI_CreateJavaVM call

2022-01-25 Thread Thomas Stuefe
On Tue, 25 Jan 2022 00:20:19 GMT, Yumin Qi wrote: > Please review, > When jlink with --compress=2, zip is used to compress the files while doing > copy. The user case failed to load zip.dll, since zip.dll is not set in PATH. > This failure is after we get NULL from

Re: RFR: 8278753: Runtime crashes with access violation during JNI_CreateJavaVM call

2022-01-25 Thread Thomas Stuefe
On Tue, 25 Jan 2022 17:22:54 GMT, Yumin Qi wrote: >> I'm curious, under what circumstances would, before >> https://bugs.openjdk.java.net/browse/JDK-8237750, we ever hit the >> LoadLibrary in imageDecompressor.cpp? Did this ever work? Was there ever a >> scenario where the JVM was not

Re: RFR: 8284642: Unexpected behavior of -XX:MaxDirectMemorySize=0

2022-04-13 Thread Thomas Stuefe
On Wed, 13 Apr 2022 12:24:46 GMT, Harold Seigel wrote: > Please review this small fix for JDK-8284642. The fix was tested by running > Mach5 tiers 1-2 on Linux, Mac OS, and Windows, and Mach5 tiers 3-5 on Linux > x64. Additionally, the modified test and the test in the bug report were run >

Re: RFR: JDK-8284874: Add comment to ProcessHandle/OnExitTest to describe zombie problem [v2]

2022-04-14 Thread Thomas Stuefe
ee comments in JBS), let > us have a clarifying comment in the test itself. Thomas Stuefe has updated the pull request incrementally with one additional commit since the last revision: modify comment - Changes: - all: https://git.openjdk.java.net/jdk/pull/8240

RFR: JDK-8284874: Add comment to ProcessHandle/OnExitTest to describe zombie problem

2022-04-14 Thread Thomas Stuefe
ProcessHandle/OnExitTest is vulnerable to misconfigured systems that do not reap zombies in a timely fashion. [JDK-8284282](https://bugs.openjdk.java.net/browse/JDK-8284282) describes this problem in detail. Until we figure out a way to fix that (if at all - see comments in JBS), let us have

Re: RFR: JDK-8284874: Add comment to ProcessHandle/OnExitTest to describe zombie problem [v3]

2022-04-14 Thread Thomas Stuefe
ee comments in JBS), let > us have a clarifying comment in the test itself. Thomas Stuefe has updated the pull request incrementally with one additional commit since the last revision: fix copyright - Changes: - all: https://git.openjdk.java.net/jdk/pull/8240

Re: RFR: JDK-8284874: Add comment to ProcessHandle/OnExitTest to describe zombie problem [v2]

2022-04-14 Thread Thomas Stuefe
On Thu, 14 Apr 2022 14:56:31 GMT, Roger Riggs wrote: > Please update the copyright date. Thanks for including the explanation in the > test. Thank you @RogerRiggs! Would I need a second Reviewer? - PR: https://git.openjdk.java.net/jdk/pull/8240

Re: RFR: 8284642: Unexpected behavior of -XX:MaxDirectMemorySize=0

2022-04-13 Thread Thomas Stuefe
On Wed, 13 Apr 2022 12:24:46 GMT, Harold Seigel wrote: > Please review this small fix for JDK-8284642. The fix was tested by running > Mach5 tiers 1-2 on Linux, Mac OS, and Windows, and Mach5 tiers 3-5 on Linux > x64. Additionally, the modified test and the test in the bug report were run >

Re: RFR: JDK-8284874: Add comment to ProcessHandle/OnExitTest to describe zombie problem [v3]

2022-04-15 Thread Thomas Stuefe
On Thu, 14 Apr 2022 18:29:21 GMT, Thomas Stuefe wrote: >> ProcessHandle/OnExitTest is vulnerable to misconfigured systems that do not >> reap zombies in a timely fashion. >> [JDK-8284282](https://bugs.openjdk.java.net/browse/JDK-8284282) describes >> this problem in

Integrated: JDK-8284874: Add comment to ProcessHandle/OnExitTest to describe zombie problem

2022-04-15 Thread Thomas Stuefe
On Thu, 14 Apr 2022 09:32:46 GMT, Thomas Stuefe wrote: > ProcessHandle/OnExitTest is vulnerable to misconfigured systems that do not > reap zombies in a timely fashion. > [JDK-8284282](https://bugs.openjdk.java.net/browse/JDK-8284282) describes > this problem in detail. > &g

Re: RFR: 8283287: ClassLoader.c cleanups [v2]

2022-03-17 Thread Thomas Stuefe
On Thu, 17 Mar 2022 16:08:11 GMT, Tyler Steele wrote: >> As mentioned in the issue, I'd like to perform the following tidying on >> ClassLoader.c >> >> - Alphabetize includes. >> - Replace 'if (ptr == 0)' with 'if (ptr == NULL)'. >> - Replace 'return 0' with 'return NULL'. > > Tyler Steele has

Re: RFR: 8286212: Cgroup v1 initialization causes NPE on some systems [v2]

2022-05-17 Thread Thomas Stuefe
On Tue, 17 May 2022 06:18:25 GMT, Ioi Lam wrote: >> Severin Gehwolf has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Use stringStream to simplify controller path assembly > > src/hotspot/os/linux/cgroupV1Subsystem_linux.cpp line 92: >

Re: RFR: 8286212: Cgroup v1 initialization causes NPE on some systems [v2]

2022-05-19 Thread Thomas Stuefe
On Wed, 18 May 2022 18:10:45 GMT, Severin Gehwolf wrote: >> @iklam yes I meant `Find the longest common prefix`. Fixed the comment. > > I'm not convinced the extra function makes the code more readable, but here > it is. I can revert back if this is too much. @jerboaa Feel free to go back to

Re: RFR: 8236569: -Xss not multiple of 4K does not work for the main thread on macOS [v2]

2022-05-31 Thread Thomas Stuefe
On Tue, 31 May 2022 15:10:38 GMT, Adam Sotona wrote: >> This is continuation of PR #4256 >> >> The patch simply rounds up the specified stack size to multiple of the >> system page size, on systems where necessary. >> The patch is based on the original PR/branch, with reflected remaining >>

Re: RFR: 8286212: Cgroup v1 initialization causes NPE on some systems [v2]

2022-05-13 Thread Thomas Stuefe
On Thu, 12 May 2022 18:09:40 GMT, Severin Gehwolf wrote: >> Please review this change to the cgroup v1 subsystem which makes it more >> resilient on some of the stranger systems. Unfortunately, I wasn't able to >> re-create a similar system as the reporter. The idea of using the longest >>

Re: RFR: 8286212: Cgroup v1 initialization causes NPE on some systems

2022-05-12 Thread Thomas Stuefe
On Tue, 10 May 2022 12:29:10 GMT, Severin Gehwolf wrote: > Please review this change to the cgroup v1 subsystem which makes it more > resilient on some of the stranger systems. Unfortunately, I wasn't able to > re-create a similar system as the reporter. The idea of using the longest >

Re: RFR: 8287363: null pointer should use NULL instead of 0

2022-05-29 Thread Thomas Stuefe
On Sat, 28 May 2022 03:31:30 GMT, Yasumasa Suenaga wrote: > We found using `0` as `NULL` in java_md_common.c . `0` is not a pointer, so > we should use `NULL` where we want to handle it. > > https://github.com/openjdk/jdk/pull/8646#discussion_r882294076 > > Also I found using `0` as NUL char

Re: RFR: 8286694: Incorrect argument processing in java launcher [v2]

2022-05-18 Thread Thomas Stuefe
On Wed, 18 May 2022 06:12:41 GMT, David Holmes wrote: >> Yasumasa Suenaga 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 >>