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

2022-02-25 Thread Thomas Stüfe
IMHO this is not hard to fix (we already have multiple proposals) and we'd should have this test on AIX too. Cheers, Thomas On Fri, Feb 25, 2022 at 6:23 PM Joe Darcy wrote: > How about excluding the test from running on AIX? > > -Joe > > On 2/25/2022 7:07 AM, Roger Riggs wrote: > > On Wed, 23

Re: Pull Request: 7013: AIX: InetAddress.getByName(addr) does not work as expected

2022-01-22 Thread Thomas Stüfe
Hi Micheal, welcome, and thank you for your contribution! I opened a bug id for you to track this: https://bugs.openjdk.java.net/browse/JDK-8280498 . I can sponsor this patch for you. You need one reviewer, I think. I don't have the cycles right now to think this through. I cc core-libs dev

Re: Performance regression in BuiltinClassLoader?

2021-01-18 Thread Thomas Stüfe
Renaming that thing would make sense. It tripped me up too when I was new to OpenJDK. ..Thomas On Mon, Jan 18, 2021 at 9:07 PM Claes Redestad wrote: > No problem :-) > > I've been advocating for renaming the /jdk intermediary into > something that would make it perfectly obvious to newcomers

Re: RFR 8252248: __SIGRTMAX is not declared in musl libc

2020-08-28 Thread Thomas Stüfe
Looks good to me. Thanks for checking. Cheers, Thomas On Fri, Aug 28, 2020 at 1:25 PM Alexander Scherbatiy < alexander.scherba...@bell-sw.com> wrote: > Hello, > > Could you review the updated fix: >http://cr.openjdk.java.net/~alexsch/8252248/webrev.01/ > > - moving shared code to

Re: RFR 8252248: __SIGRTMAX is not declared in musl libc

2020-08-26 Thread Thomas Stüfe
Hi Alexander, --- src/java.base/unix/native/libnet/net_util_md.h why include pthread.h? SIGRTMAX is in signal.h on posix ( https://pubs.opengroup.org/onlinepubs/007908799/xsh/signal.h.html). I see the old code included this too, pretty sure this was either incidental or just wrong. --- Are

Re: RFR(M): 8249963: Make the zlib implementation selectively configurable at startup

2020-07-23 Thread Thomas Stüfe
Hi, Could this not be handled like any other third party provided binary which can run in our process? Say, a jvmti agent, any JNI library, or any system library? For all these cases we expect the user to know what he is doing, resp. the system library not to be buggy. Why not here? This seems

Re: RFR 8249217: Unexpected StackOverflowError in "process reaper" thread still happens

2020-07-22 Thread Thomas Stüfe
Hi, On Thu, Jul 23, 2020 at 6:01 AM Roger Riggs wrote: > Hi David, > > Yep, that's the focal point. > Its both debug and -Xcomp. > There must be something that changes the timing or the synchronization > or the sizes; but that's more than I know about how the VM works. > Just a really wild

Re: RFR (XS) 8245722: 32-bit build failures after JDK-8243491

2020-05-25 Thread Thomas Stüfe
Seems fine and trivial. ..Thomas On Mon 25. May 2020 at 16:09, Aleksey Shipilev wrote: > On 5/25/20 2:38 PM, Aleksey Shipilev wrote: > > Bug: > > https://bugs.openjdk.java.net/browse/JDK-8245722 > > > > This is similar to JDK-8236634, and fix takes the same form: > > > > diff -r d52c2e540934

Re: RFR [XXS] : 8244183: linker error jpackageapplauncher on Windows 32bit

2020-05-01 Thread Thomas Stüfe
You have enough reviews now, but thanks for fixing this. ..Thomas On Thu, Apr 30, 2020 at 4:15 PM Baesken, Matthias wrote: > Hello, please look into this small fix for a link error we faced on > Windows 32bit. > > Currently we run into this link error : > jpackageapplauncher > > * For target >

Re: RFR(S) 8241071 Generation of classes.jsa with -Xshare:dump is not deterministic

2020-04-28 Thread Thomas Stüfe
p objects, which I also fixed. > > > > http://cr.openjdk.java.net/~iklam/jdk15/8241071-deterministic-cds-archive.v03/ > > http://cr.openjdk.java.net/~iklam/jdk15/8241071-deterministic-cds-archive.v03.delta/ > > Thanks > - Ioi > > > On 4/27/20 2:14 AM, Thomas

Re: RFR(S) 8241071 Generation of classes.jsa is not deterministic

2020-04-27 Thread Thomas Stüfe
, Thomas On Mon, Apr 27, 2020 at 9:58 AM Thomas Stüfe wrote: > Hi Ioi, > > Please don't do this :) > > First off, how would this work when dumping with > UseCompressedClassPointers off? In that case allocation would be relegated > to non-class metaspace which cannot guarantee

Re: RFR(S) 8241071 Generation of classes.jsa is not deterministic

2020-04-27 Thread Thomas Stüfe
On Mon, Apr 27, 2020 at 8:23 AM David Holmes wrote: > Hi Ioi, > > On 27/04/2020 3:22 pm, Ioi Lam wrote: > > https://bugs.openjdk.java.net/browse/JDK-8241071 > > > http://cr.openjdk.java.net/~iklam/jdk15/8241071-deterministic-cds-archive.v02/ > > > > > > The goal is to for "java -Xshare:dump" to

Re: RFR(S) 8241071 Generation of classes.jsa is not deterministic

2020-04-27 Thread Thomas Stüfe
Hi Ioi, Please don't do this :) First off, how would this work when dumping with UseCompressedClassPointers off? In that case allocation would be relegated to non-class metaspace which cannot guarantee that kind of address stability. Even in class space, I do not think you can guarantee

Re: RFR: 8239365: ProcessBuilder/Basic.java test modifications for AIX execution

2020-04-22 Thread Thomas Stüfe
sor and merge, please? > > Best Regards > > Adam Farley > IBM Runtimes > > > "Thomas Stüfe" wrote on 21/04/2020 18:29:23: > > > From: "Thomas Stüfe" > > To: Adam Farley8 > > Cc: core-libs-dev , Roger Riggs > > > > Date:

Re: RFR: 8239365: ProcessBuilder/Basic.java test modifications for AIX execution

2020-04-21 Thread Thomas Stüfe
it passes when it should pass, and fails only when it should > fail, clarity is a "would be nice", but ultimately a secondary priority. > > Thanks for your feedback. :) > > Best Regards > > Adam Farley > IBM Runtimes > > > "Thomas Stüfe" wrote on 03/03

Re: RFR(S): 8242848: Improve performance of InflaterOutputStream.write()

2020-04-21 Thread Thomas Stüfe
ay return even if finished() is not true. > > Yes, that's true, but that's what we must do if there's no more input > available. Before my change this break on "len < 1" was in the "if > (inf.needsInput())" branch. > > On Thu, Apr 16, 2020 at 8:22

Re: RFR(S): 8242848: Improve performance of InflaterOutputStream.write()

2020-04-16 Thread Thomas Stüfe
As for increasing the buffer size, it makes sense but IMHO needs a CSR and a release note. Cheers, Thomas On Thu, Apr 16, 2020 at 8:21 AM Thomas Stüfe wrote: > Hi Volker, > > 252 // Check the decompressor > 253 if (inf.finished() || (len == 0)/* n

Re: RFR(S): 8242848: Improve performance of InflaterOutputStream.write()

2020-04-16 Thread Thomas Stüfe
Hi Volker, 252 // Check the decompressor 253 if (inf.finished() || (len == 0)/* no more input */) { 254 break; 255 } Not sure but I think this is wrong because now you bypass the followup handling of inf.needsDirectory.

Re: [PING] RFR(s): 8176894 Provide specialized implementation for default methods putIfAbsent, computeIfAbsent, computeIfPresent, compute, merge in TreeMap

2020-03-29 Thread Thomas Stüfe
On Sun 29. Mar 2020 at 18:48, Martin Buchholz wrote: > I'm fine with the latest version > > On Sun, Mar 29, 2020 at 1:29 AM Tagir Valeev wrote: > >> >> > At various places, the "@throws ConcurrentModificationException" >> javadoc: >> > >> > 619 * @throws ConcurrentModificationException if

Re: RFR: 8232846: ProcessHandle.Info command with non-English shows question marks

2020-03-27 Thread Thomas Stüfe
Looks good to me in its current form, thanks. On Fri, Mar 27, 2020 at 8:02 AM Toshio 5 Nakamura wrote: > Hi Thomas, > > Yes, I tested it locally with small buffer size, 50 for example. But, it's > hard to create a jtreg test. > > I confirmed ERROR_INSUFFICIENT_BUFFER returned from

Re: RFR: 8232846: ProcessHandle.Info command with non-English shows question marks

2020-03-27 Thread Thomas Stüfe
Hi Toshio, did you test the malloc path? Since https://docs.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-queryfullprocessimagenamew#return-value does not mention ERROR_INSUFFICIENT_BUFFER as return code, would like to see this working. It does however mention that it returns the

Re: RFR: 8232846: ProcessHandle.Info command with non-English shows question marks

2020-03-26 Thread Thomas Stüfe
Hi Toshio, looks good to me. Cheers, Thomas On Thu, Mar 26, 2020 at 9:58 AM Toshio 5 Nakamura wrote: > > Hi All, > > Could you review this change? Additionally, I'd like to ask a sponsor for > it, since I'm not a committer. > > Bug: https://bugs.openjdk.java.net/browse/JDK-8232846 > Webrev:

Re: [PING] RFR(s): 8176894 Provide specialized implementation for default methods putIfAbsent, computeIfAbsent, computeIfPresent, compute, merge in TreeMap

2020-03-25 Thread Thomas Stüfe
On Tue, Mar 24, 2020 at 10:25 PM Florian Weimer wrote: > * Thomas Stüfe: > > > Hi Tagir, > > > > nice work. Only a partwise review for TreeMap.java, did not yet look at > the > > tests. > > > > remapValue: > > > > 711 } el

Re: [PING] RFR(s): 8176894 Provide specialized implementation for default methods putIfAbsent, computeIfAbsent, computeIfPresent, compute, merge in TreeMap

2020-03-24 Thread Thomas Stüfe
Hi Tagir, nice work. Only a partwise review for TreeMap.java, did not yet look at the tests. remapValue: 711 } else { 712 // replace old mapping 713 t.value = newValue; 714 return newValue; 715 } Should we increase the modification count

Re: RFR: 8239365: ProcessBuilder/Basic.java test modifications for AIX execution

2020-03-03 Thread Thomas Stüfe
This is why I always was against handing up the result of strerror to the user :) The same problem we would have when running with different locales. We should have a platform agnostic string table in the java lib for that purpose... As for the test, looks good, but I personally would shorten the

Re: RFR(XS): 8239856: [ntintel] asserts about copying unaligned array element

2020-02-24 Thread Thomas Stüfe
e/libjdwp/ArrayReferenceImpl.c. > > > > Christoph had already suggested to make it available for core libs, too, > but I haven’t found a good place for it. > > > > Best regards, > > Martin > > > > > > *From:* Thomas Stüfe > *Sent:* Montag, 24. Februar 2

Re: RFR(XS): 8239856: [ntintel] asserts about copying unaligned array element

2020-02-24 Thread Thomas Stüfe
Hi Martin, maybe use ATTRIBUTE_ALIGNED instead? Cheers, Thomas On Mon, Feb 24, 2020 at 12:44 PM Doerr, Martin wrote: > Hi, > > > > we had fixed stack array alignment for Windows 32 bit with JDK-8220348. > > However, there are also stack allocated jlong and jdouble used as source > for

Re: RFR: 8239351: Give more meaningful InternalError messages in Deflater.c

2020-02-19 Thread Thomas Stüfe
+1 Gruß, Thomas On Wed, Feb 19, 2020, 10:35 Baesken, Matthias wrote: > Hello Thomas / Lance / Martin, thanks for the reviews . > > I added a little helper function, new webrev : > > > > http://cr.openjdk.java.net/~mbaesken/webrevs/8239351.1/ > > > > Best regards, Matthias > > > > > > > > I

Re: RFR: 8239351: Give more meaningful InternalError messages in Deflater.c

2020-02-18 Thread Thomas Stüfe
I like this too. +1 for factoring out throwing the error. ..Thomas On Tue, Feb 18, 2020 at 6:51 PM Martin Buchholz wrote: > Thanks for doing this. Looks good to me. > I would probably create a tiny helper function to encapsulate the error > throw. > > On Tue, Feb 18, 2020 at 7:03 AM Baesken,

Re: RFR: JDK-8238380: java.base/unix/native/libjava/childproc.c "multiple definition" link errors with GCC10

2020-02-05 Thread Thomas Stüfe
> > Regards > > Patrick > > > > *From:* Thomas Stüfe > *Sent:* Wednesday, February 5, 2020 11:16 PM > *To:* Patrick Zhang OS > *Cc:* core-libs-dev > *Subject:* Re: RFR: JDK-8238380: > java.base/unix/native/libjava/childproc.c "multiple definition" link err

Re: RFR: JDK-8238380: java.base/unix/native/libjava/childproc.c "multiple definition" link errors with GCC10

2020-02-05 Thread Thomas Stüfe
ion > to fix the build errors purely, and try best to avoid any side effect. So I > would leave the further improvement to others, and keep it as-is. Thanks. > > > > Updated the change a bit (the header comments): > http://cr.openjdk.java.net/~qpzhang/8238380/webrev.02/ > >

Re: RFR: JDK-8238380: java.base/unix/native/libjava/childproc.c "multiple definition" link errors with GCC10

2020-02-04 Thread Thomas Stüfe
Hi Patrick, You patch looks alright. But I wonder whether a cleaner solution would not be to add a parentPathv pointer to the ChildStuff structure and pass it down to JDK_execvpe that way, like we do for all other input arguments of that function. If it is an input argument, seems strange to

Re: New candidate JEP: 371: Hidden Classes

2020-01-27 Thread Thomas Stüfe
On Sat, Jan 25, 2020 at 6:09 PM Mandy Chung wrote: > > > On 1/24/20 9:15 PM, Bernd Eckenfels wrote: > > Hello, > > > > I wonder will this (weak class) be useful for reflective method > > accessors and even be able to reduce/remove the need of > > jdk.internal.reflect.DelegatingClassLoaders? If

Re: RFR(S): 8220348: [ntintel] asserts about copying unalinged array

2019-12-04 Thread Thomas Stüfe
Hi Martin, this makes sense. This is the right way to force alignment. I do not like the platform code in the shared file but do not think this is a big deal. +#if defined (_WIN32) && defined (_MSC_VER) Why do you think we need _MSC_VER too? Is OpenJDK on Windows even buildable with anything

Re: RFR (L, final): 8218626: Add detailed message to NullPointerException describing what is null.

2019-09-17 Thread Thomas Stüfe
s an immutable string, but > for trim_well_known_class_names this does not work. > So I'd propose this: > http://cr.openjdk.java.net/~goetz/wr19/8218628-exMsg-NPE/17-incremental/ > http://cr.openjdk.java.net/~goetz/wr19/8218628-exMsg-NPE/17/ > > Best regards, > Goetz. > > > -Original Mess

Re: RFR (L, final): 8218626: Add detailed message to NullPointerException describing what is null.

2019-09-17 Thread Thomas Stüfe
Additionally, since 8224193, stringStream does not use RA anymore, so you do not need ResourceMarks for the backing buffer. 8224193 has been backported to 11, btw. On Mon, Sep 16, 2019 at 2:53 PM Thomas Stüfe wrote: > Hi Goetz, > > not a full review, just a small suggestion. In jv

Re: RFR (L, final): 8218626: Add detailed message to NullPointerException describing what is null.

2019-09-16 Thread Thomas Stüfe
Hi Goetz, not a full review, just a small suggestion. In jvm.cpp you could just access ss->base() instead of ss->as_string() since the internal buffer is already NULL terminated and result_string does not outlive the stringStream object. Also it misses including ostream.hpp. Cheers, Thomas On

RFR(T): 8230910: libsspi_bridge does not build on Windows 32bit

2019-09-12 Thread Thomas Stüfe
Hi all, may I please have reviews for the following trivial build fix. Bug: https://bugs.openjdk.java.net/browse/JDK-8230910 webrev: http://cr.openjdk.java.net/~stuefe/webrevs/8230910-libsspi_bridge_does_not_build_on_windows_32bit/webrev.00/webrev/ Thanks, Thomas

Re: RFR(T): 8227252: [aix] Disable jdk/java/lang/reflect/exeCallerAccessTest

2019-07-04 Thread Thomas Stüfe
- > > From: core-libs-dev On Behalf > Of > > Thomas Stüfe > > Sent: Donnerstag, 4. Juli 2019 11:14 > > To: Java Core Libs ; ppc-aix-port-dev > > aix-port-...@openjdk.java.net> > > Subject: RFR(T): 8227252: [aix] Disable > > jdk/java/lang/reflect

RFR(T): 8227252: [aix] Disable jdk/java/lang/reflect/exeCallerAccessTest

2019-07-04 Thread Thomas Stüfe
Greetings, please review this trivial fix which switches off jdk/java/lang/reflect/exeCallerAccessTest on AIX. JBS: https://bugs.openjdk.java.net/browse/JDK-8227252 cr: http://cr.openjdk.java.net/~stuefe/webrevs/8227252-disable-execallertest-on-aix/webrev.00/webrev/ Thanks, Thomas

Re: 8226863: [aix] jdk/java/lang/reflect/exeCallerAccessTest cannot launch on primordial thread

2019-07-02 Thread Thomas Stüfe
ndy > > > > On 7/1/19 10:23 PM, Thomas Stüfe wrote: > > Second, corrected Webrev: > > > > > http://cr.openjdk.java.net/~stuefe/webrevs/execalleraccesstest-cannot-launch-on-primordial-thread/webrev.01/webrev/ > > > > Run through SAP nightlies on all platforms

Re: 8226863: [aix] jdk/java/lang/reflect/exeCallerAccessTest cannot launch on primordial thread

2019-07-01 Thread Thomas Stüfe
Second, corrected Webrev: http://cr.openjdk.java.net/~stuefe/webrevs/execalleraccesstest-cannot-launch-on-primordial-thread/webrev.01/webrev/ Run through SAP nightlies on all platforms. Cheers, Thomas On Thu, Jun 27, 2019 at 9:02 AM Thomas Stüfe wrote: > Hi all, > > Issue

Re: 8226863: [aix] jdk/java/lang/reflect/exeCallerAccessTest cannot launch on primordial thread

2019-06-28 Thread Thomas Stüfe
*The only common part of the > changes would be to rename main to main_inner and call this method for all > non-AIX cases… > Yea, I'll repost an update next week. Thanks for the review. Cheers, Thomas > > Best regards > > Christoph > > > > *From:* ppc-aix-port-dev *On &g

8226863: [aix] jdk/java/lang/reflect/exeCallerAccessTest cannot launch on primordial thread

2019-06-27 Thread Thomas Stüfe
Hi all, Issue: https://bugs.openjdk.java.net/browse/JDK-8226863 webrev: http://cr.openjdk.java.net/~stuefe/webrevs/8226863--jdk-java-lang-reflect-execalleraccesstest-cannot-launch-on-primordial-thread/webrev.00/webrev/ we have this annoying issue on AIX that the libjvm cannot be invoked on a

Re: [11u]: RFR: 8215281: Use String.isEmpty() when applicable in java.base

2019-06-24 Thread Thomas Stüfe
as received enough reviewer attention. Will push > it then as regression tests seem all good, too.  > > Thanks to all involved, > Christoph > > > > -Original Message- > > From: Claes Redestad > > Sent: Montag, 24. Juni 2019 10:24 > > To: Thomas

Re: [11u]: RFR: 8215281: Use String.isEmpty() when applicable in java.base

2019-06-24 Thread Thomas Stüfe
HI Claes, On Mon, Jun 24, 2019 at 10:22 AM Claes Redestad wrote: > Hi, > > > On 2019-06-24 09:27, Thomas Stüfe wrote: > > However, your patch (and the original one) contained the following part I > > do not understand: > > > > > http://cr.openjdk.ja

Re: [11u]: RFR: 8215281: Use String.isEmpty() when applicable in java.base

2019-06-24 Thread Thomas Stüfe
Hi Christoph, I looked at the changes. Mostly good - true to the original change. At various places (e.g. src/java.base/share/classes/java/lang/ClassLoader.java) one could remove the extra brackets, but since we want to keep the diff to head at a minimum where possible I would not do this.

Re: Question re: Usage of JNICALL with varargs function with 32bit compilers

2019-06-18 Thread Thomas Stüfe
Hi Steve, I always saw this as a harmless warning, safe to ignore, since both caller and callee are forced back to the same convention so it all works out in the end. Or can you envision a scenario where this would be harmfull? Thank god Microsoft abandoned this calling convention circus with

Re: 8226242 : Diagnostic output for posix_spawn failure

2019-06-17 Thread Thomas Stüfe
s of the status > but a bit easier to read than the hex formatted value. > If there is any other information available at that point of execution, > let me know. > > Thanks, Roger > > [1] > http://cr.openjdk.java.net/~rriggs/webrev-spawn-diag-8225192-1/index.html > > On

Re: 8226242 : Diagnostic output for posix_spawn failure

2019-06-17 Thread Thomas Stüfe
Hi Roger, thanks for taking care of this! I think WIFSIGNALED is just a bool though; to get the terminating signal, you need to use WTERMSIG. Strictly speaking, WEXISTSTATUS is only valid if WIFEXITED!=0 and WTERMSIG only if WIFSIGNALLED!=0; I think they are undefined otherwise. Cheers, Thoams

Re: PING: RFR(s): (new approach) 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error

2019-06-05 Thread Thomas Stüfe
On Wed, Jun 5, 2019 at 8:37 AM Martin Buchholz wrote: > > > On Tue, Jun 4, 2019 at 9:50 PM Thomas Stüfe > wrote: > >> >> I would vote for keeping the name as it is: I would have to change a >> number of places, and when in doubt I'd like to keep the changes mini

Re: PING: RFR(s): (new approach) 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error

2019-06-05 Thread Thomas Stüfe
shell files. >> >> And yes, count me as a Reviewer and reviewed. >> >> Roger >> >> On 06/04/2019 12:14 PM, Thomas Stüfe wrote: >> >> On Tue, Jun 4, 2019 at 5:09 PM Roger Riggs >> wrote: >> >

Re: RFR(xs): 8224181: On child process spawn, child may write to random file descriptor instead of the fail pipe

2019-06-05 Thread Thomas Stüfe
_fd below (that also > won't make a difference in practice) > > 386 close(FAIL_FILENO); > > > > On Tue, Jun 4, 2019 at 11:01 PM Thomas Stüfe > wrote: > >> Hi Martin, >> >> can you quickly chime in on this, this is a very small patch and I'd like

Re: PING: RFR(s): (new approach) 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error

2019-06-04 Thread Thomas Stüfe
On Wed, Jun 5, 2019 at 5:06 AM Martin Buchholz wrote: > > > On Tue, Jun 4, 2019 at 2:01 PM Roger Riggs wrote: > >> Hmm. Can_JOHNNY_EXEC naming is a bit quirky, not intuitive... >> > > I'm the original author of that joke^H deeply evocative symbol name, so I > might be biased. > > I like the

Re: PING: RFR(s): (new approach) 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error

2019-06-04 Thread Thomas Stüfe
Hi Florian, Interesting, but in this case it is not posix_spawn, but plain fork(). The VM does this, the exec calls come from us, not the libc. See childproc.c . Cheers, Thomas’ On Tue 4. Jun 2019 at 22:42, Florian Weimer wrote: > * Thomas Stüfe: > > > Then I ran an strace ove

Re: PING: RFR(s): (new approach) 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error

2019-06-04 Thread Thomas Stüfe
t then > passes it as > the first argument to /bin/sh that will handle the shell files. > > And yes, count me as a Reviewer and reviewed. > > Roger > > On 06/04/2019 12:14 PM, Thomas Stüfe wrote: > > On Tue, Jun 4, 2019 at 5:09 PM Roger Riggs wrote: > > ... > &

Re: PING: RFR(s): (new approach) 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error

2019-06-04 Thread Thomas Stüfe
On Tue, Jun 4, 2019 at 7:25 PM Martin Buchholz wrote: > > > On Tue, Jun 4, 2019 at 8:09 AM Roger Riggs wrote: > >> Hi Thomas, >> >> A minor concern is the impact of the extra write and read that can cause >> rescheduling >> of the parent and child processes. But that's probably in the noise >>

Re: PING: RFR(s): (new approach) 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error

2019-06-04 Thread Thomas Stüfe
agree on the complexity remark. I do not really like it either. Maybe we can remove this fix again if in the far future all posix_spawn() variants in the wild report exec errors back like they should. I believe Florian added already a patch to the glibc? Thanks, Thomas > On Mon, Jun 3, 2019 at 11

Re: PING: RFR(s): (new approach) 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error

2019-06-04 Thread Thomas Stüfe
we do this? > > Thanks, Roger > > Thank you Roger. Can I consider the patch to be reviewed by you? ..Thomas > On 06/04/2019 02:06 AM, Thomas Stüfe wrote: > > Hi all, > > may I please have reviews/opinions on this fix? > > Fix has been live in our test

Re: RFR(xs): 8224181: On child process spawn, child may write to random file descriptor instead of the fail pipe

2019-06-04 Thread Thomas Stüfe
Thank you Roger! On Tue, Jun 4, 2019 at 5:13 PM Roger Riggs wrote: > H Thomas, > > Looks ok. > > Roger > > > On 06/04/2019 09:23 AM, Thomas Stüfe wrote: > > Hi all, > > > > may I please have reviews for this small fix: > > > > Bug: h

RFR(xs): 8224181: On child process spawn, child may write to random file descriptor instead of the fail pipe

2019-06-04 Thread Thomas Stüfe
Hi all, may I please have reviews for this small fix: Bug: https://bugs.openjdk.java.net/browse/JDK-8224181 cr: http://cr.openjdk.java.net/~stuefe/webrevs/8224181-on-child-process-spawn--child-may-write-to-wrong-file-descriptor-instead-of-the-fail-pipe/webrev.00/webrev/ In sub process error

PING: RFR(s): (new approach) 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error

2019-06-04 Thread Thomas Stüfe
Hi all, may I please have reviews/opinions on this fix? Fix has been live in our test landscape since some weeks. If we do not want this fix to be in JDK13, we may want to revert the posix_spawn-by-default-on-Linux change. Thanks, Thomas On Mon, May 20, 2019 at 4:15 PM Thomas Stüfe wrote

Re: RFR(s): (new approach) 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error

2019-05-22 Thread Thomas Stüfe
nalyze the change in detail, and I may not get time to do so. > But I'm not a reviewer in any case, so maybe that doesn't matter too > much. > > On Wed, May 22, 2019 at 1:16 AM Thomas Stüfe > wrote: > > > > Ping... > > > > Guys, I need some feedback on this. If we

Re: RFR(s): (new approach) 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error

2019-05-22 Thread Thomas Stüfe
No problems in tier 1 tests (jdk-submit). ..Thomas On Wed, May 22, 2019 at 8:16 AM Thomas Stüfe wrote: > Ping... > > Guys, I need some feedback on this. If we do not fix this issue, we may > want to roll back the use of posix_spawn() as a default and return to vfork > for JDK

Re: RFR(s): (new approach) 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error

2019-05-22 Thread Thomas Stüfe
, 2019 at 4:15 PM Thomas Stüfe wrote: > Hi all, > > (old mail thread: > https://mail.openjdk.java.net/pipermail/core-libs-dev/2019-May/060139.html > ) > > May I please have your reviews and opinions for the following bug fix: > > issue: https://bugs.openjdk.java.net/brow

Re: RFR(s): (new approach) 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error

2019-05-20 Thread Thomas Stüfe
Hi Florian, On Mon, May 20, 2019 at 4:24 PM Florian Weimer wrote: > * Thomas Stüfe: > > > Hi all, > > > > (old mail thread: > https://mail.openjdk.java.net/pipermail/core-libs-dev/2019-May/060139.html > ) > > > > May I please have your revi

RFR(s): (new approach) 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error

2019-05-20 Thread Thomas Stüfe
Hi all, (old mail thread: https://mail.openjdk.java.net/pipermail/core-libs-dev/2019-May/060139.html) May I please have your reviews and opinions for the following bug fix: issue: https://bugs.openjdk.java.net/browse/JDK-8223777 cr:

Re: RFR (T): 8224042 Add private alignDown method to MappedByteBuffer

2019-05-16 Thread Thomas Stüfe
Hi Andrew, looks good and trivial. Method could be static. Cheers, Thomas On Thu, May 16, 2019 at 4:49 PM Andrew Dinn wrote: > Please review this trivial change to MapperByteBuffer which encapsulates > the page align down operation in a suitably named method. > > JIRA:

Re: RFR(s): 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error

2019-05-15 Thread Thomas Stüfe
ue, May 14, 2019 at 9:00 PM Florian Weimer wrote: > * Thomas Stüfe: > > > Right now I am worried more about things I cannot determine yet. Where > > before we would wait for the pipe to get broken, now we have a read > > call on the parent side, a write call on the child si

Re: RFR(s): 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error

2019-05-14 Thread Thomas Stüfe
On Tue, May 14, 2019 at 2:16 PM Florian Weimer wrote: > * David Lloyd: > > > Pipe communication isn't very costly AFAIK. The added complexity > > would be waiting for either a thing to appear on the pipe indicating > > success or else a waitpid/waitid+WNOHANG for exit code 127. But > > writing

Re: RFR(s): 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error

2019-05-14 Thread Thomas Stüfe
Neat idea! But this incurs costs for this pipe communication on every spawn. What do the others think? Cheers, Thomas On Tue, May 14, 2019 at 10:22 AM Florian Weimer wrote: > * Thomas Stüfe: > > > This is impossible to fix completely - see Martin's comment about the >

Re: RFR(s): 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error

2019-05-14 Thread Thomas Stüfe
the literal "jspawnhelper" which would be > misleading if it were > changed and it might be clearer to mention 'exec' permission in the > message. > > Done. In the message I talk now about "spawn helper" and foresee many a blog post explaining how to fix this :P

RFR(s): 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error

2019-05-13 Thread Thomas Stüfe
Hi all, may I have please your opinions about the following change: Bug: https://bugs.openjdk.java.net/browse/JDK-8223777 webrev: http://cr.openjdk.java.net/~stuefe/webrevs/8223777-posix_spawn-no-exec-error/webrev.00/webrev/ In short, since some weeks posix_spawn() is used on Linux to spawn

Re: Process.exec with the linux posix_spawn mode has a bug

2019-05-13 Thread Thomas Stüfe
On Mon, May 13, 2019 at 4:11 PM Thomas Stüfe wrote: > > > On Mon, May 13, 2019 at 3:42 PM Thomas Stüfe > wrote: > >> >> Hi Martin, >> >> On Mon, May 13, 2019 at 2:08 PM Martin Buchholz >> wrote: >> >>> >>> >>> I a

Re: Process.exec with the linux posix_spawn mode has a bug

2019-05-13 Thread Thomas Stüfe
On Mon, May 13, 2019 at 3:42 PM Thomas Stüfe wrote: > > Hi Martin, > > On Mon, May 13, 2019 at 2:08 PM Martin Buchholz > wrote: > >> >> >> I am happy this is resolved and the intermittent behavior explained. Yes, >>> we could improve except

Re: Process.exec with the linux posix_spawn mode has a bug

2019-05-13 Thread Thomas Stüfe
On Mon, May 13, 2019 at 4:08 PM Martin Buchholz wrote: > >>> I tried hard back in 2005 to provide pretty good java-level diagnostics >>> when subprocess starting failed somehow (see WhyCantJohnnyExec) . At least >>> the errno did get reported. >>> >>> >> I know your code. For many years I

Re: Process.exec with the linux posix_spawn mode has a bug

2019-05-13 Thread Thomas Stüfe
Hi Martin, On Mon, May 13, 2019 at 2:08 PM Martin Buchholz wrote: > > > I am happy this is resolved and the intermittent behavior explained. Yes, >> we could improve exception messages, especially since analyzing fork >> scenarios is cumbersome. >> > > I tried hard back in 2005 to provide

Re: Process.exec with the linux posix_spawn mode has a bug

2019-05-12 Thread Thomas Stüfe
wrote: > Thanks Thomas, > by the way, what is the purpose of jexec ? > > Rémi > > -- > > *De: *"Thomas Stüfe" > *À: *"David Lloyd" > *Cc: *"Remi Forax" , "core-libs-dev" < > core-libs

Re: Process.exec with the linux posix_spawn mode has a bug

2019-05-11 Thread Thomas Stüfe
Hi Remi, David is right - a few more details: In both fork- and vfork-mode we fork/vfork the child process and in the child process then exec() the target binary (in this case "java" itself). In posix_spawn mode, for complicated reasons, we start, via posix_spawn(), the jspawnhelper - which,

Re: RFR: 8221477: Inject os/cpu-specific constants into Unsafe from JVM

2019-04-05 Thread Thomas Stüfe
Still looking good :) ..Thomas On Thu, Apr 4, 2019 at 5:19 PM Andrew Dinn wrote: > New webrev is now available. Re-reviews welcome. > > JIRA: https://bugs.openjdk.java.net/browse/JDK-8221477 > Webrev: http://cr.openjdk.java.net/~adinn/8221477/webrev.03 > > This version should address all

Re: RFR: 8221477: Inject os/cpu-specific constants into Unsafe from JVM

2019-03-29 Thread Thomas Stüfe
Thanks Andrew. Looks all good to me now. Cheers, Thomas On Fri, Mar 29, 2019 at 9:43 AM Andrew Dinn wrote: > On 28/03/2019 17:09, Thomas Stüfe wrote: > > src/hotspot/share/classfile/javaClasses.cpp > > > > Pure nitpicking, but you could remove the member variables

Re: RFR: 8221477: Inject os/cpu-specific constants into Unsafe from JVM

2019-03-28 Thread Thomas Stüfe
On Thu, Mar 28, 2019 at 5:56 PM Andrew Dinn wrote: > On 28/03/2019 15:22, Thomas Stüfe wrote: > > The second of those was actually intended to be iff. This is a common > > abbreviation used by English/US mathematicians and logicians to write > > 'if and only if'

Re: RFR: 8221477: Inject os/cpu-specific constants into Unsafe from JVM

2019-03-28 Thread Thomas Stüfe
Btw congrats for finally getting JEP 352 to move on. That really took a while. Cheers, Thomas On Thu, Mar 28, 2019 at 4:22 PM Thomas Stüfe wrote: > > > On Thu, Mar 28, 2019 at 3:41 PM Andrew Dinn wrote: > >> On 28/03/2019 12:17, Thomas Stüfe wrote: >> > this looks f

Re: RFR: 8221477: Inject os/cpu-specific constants into Unsafe from JVM

2019-03-28 Thread Thomas Stüfe
On Thu, Mar 28, 2019 at 3:41 PM Andrew Dinn wrote: > On 28/03/2019 12:17, Thomas Stüfe wrote: > > this looks fine, nits only: > > Thank you for the review, Thomas. I'll post a follow up webrev to > address the comments. Responses are inline. > > > UnsafeConstantsFi

Re: RFR: 8221477: Inject os/cpu-specific constants into Unsafe from JVM

2019-03-28 Thread Thomas Stüfe
Hi, this looks fine, nits only: UnsafeConstantsFixup::do_field() you could shrink that coding a bit by factoring out the checks, e.g.: static oop mirror_with_checks_from_field_descriptor(fieldDescriptor* fd) { oop mirror = fd->field_holder()->java_mirror(); assert(fd->field_holder() ==

Re: RFR(xs): 8221375: Windows 32bit build (VS2017) broken

2019-03-27 Thread Thomas Stüfe
at you guys are regularly building it (if possible fastdebug too). Please report any build breaks. When reported promptly, with the offending change linked in JBS, the authors will often try to fix it themselves. Cheers, Thomas > Cheers, > Martijn > > > On Tue, 26 Mar 2019 at 06:04,

Re: RFR (XS) 8221524: java/util/Base64/TestEncodingDecodingLength.java should be disabled on 32-bit platforms

2019-03-27 Thread Thomas Stüfe
Good & trivial. ..Thomas On Wed, Mar 27, 2019 at 12:00 AM Aleksey Shipilev wrote: > Bug: > https://bugs.openjdk.java.net/browse/JDK-8221524 > > It currently requests -Xmx8g, which is above the max representable memory > for 32-bit VM. The test > allocates byte[Integer.MAX_VALUE - C], which

Re: RFR (XS) 8221401: [TESTBUG] java/math/BigInteger/LargeValueExceptions.java should be disabled on 32-bit platforms

2019-03-26 Thread Thomas Stüfe
Looks fine and trivial. ..Thomas On Tue, Mar 26, 2019 at 2:02 PM Aleksey Shipilev wrote: > Ping, no takers? > > -Aleksey > > On 3/25/19 3:33 PM, Aleksey Shipilev wrote: > > Bug: > > https://bugs.openjdk.java.net/browse/JDK-8221401 > > > > My major concern is to make tier1 passing on x86_32.

Re: RFR(xs): 8221375: Windows 32bit build (VS2017) broken

2019-03-26 Thread Thomas Stüfe
On Mon, Mar 25, 2019 at 10:49 PM Martijn Verburg wrote: > Hi all, > > Snipping much, but commenting on one statement inline below. > > On Mon, 25 Mar 2019 at 01:58, David Holmes > wrote: > >> Hi Thomas, >> >> A few queries, comments and concerns ... &g

Re: RFR (XS) 8221400: [TESTBUG] java/lang/String/StringRepeat.java requests too much heap

2019-03-25 Thread Thomas Stüfe
On Mon, Mar 25, 2019 at 3:47 PM Aleksey Shipilev wrote: > On 3/25/19 2:40 PM, Thomas Stüfe wrote: > > Note that this may not be enough for windows 32bit. Highest I can go > there (win32 slowdebug build I > > just built) is 1400m. > > That probably means something els

Re: RFR (XS) 8221400: [TESTBUG] java/lang/String/StringRepeat.java requests too much heap

2019-03-25 Thread Thomas Stüfe
Looks fine and trivial. Note that this may not be enough for windows 32bit. Highest I can go there (win32 slowdebug build I just built) is 1400m. Cheers, Thomas On Mon, Mar 25, 2019 at 3:33 PM Aleksey Shipilev wrote: > Bug: > https://bugs.openjdk.java.net/browse/JDK-8221400 > > My major

RFR(xs): 8221375: Windows 32bit build (VS2017) broken

2019-03-24 Thread Thomas Stüfe
Hi all, After a long time I tried to build a Windows 32bit VM (VS2017) and failed; multiple errors and warnings. Lets reverse the bitrot: cr: http://cr.openjdk.java.net/~stuefe/webrevs/8221375--windows-32bit-build-(vs2017)-broken-in-many-places/webrev.00/webrev/ Issue:

Re: [11u]: RFR CSR backport: JDK-8220362: Add "POSIX_SPAWN" as valid value to jdk.lang.Process.launchMechanism on Linux

2019-03-20 Thread Thomas Stüfe
On Tue, Mar 19, 2019 at 5:10 PM Martin Buchholz wrote: > I'm the one who introduced vfork for process spawning, and I support > Thomas' plan to switch to posix_spawn. > (although I have not seen a crash attributed to using vfork on Linux) > Thanks Martin. I think for a long time vfork was the

Re: [11u]: RFR CSR backport: JDK-8220362: Add "POSIX_SPAWN" as valid value to jdk.lang.Process.launchMechanism on Linux

2019-03-19 Thread Thomas Stüfe
Thank you, Andrew .. Thomas On Tue, Mar 19, 2019, 5:42 PM Andrew Haley wrote: > On 3/19/19 4:10 PM, Martin Buchholz wrote: > > I'm the one who introduced vfork for process spawning, and I support > Thomas' plan to switch to posix_spawn. > > (although I have not seen a crash attributed to using

Re: [11u]: RFR CSR backport: JDK-8220362: Add "POSIX_SPAWN" as valid value to jdk.lang.Process.launchMechanism on Linux

2019-03-19 Thread Thomas Stüfe
On Tue, Mar 19, 2019 at 10:39 AM Andrew Haley wrote: > On 3/19/19 7:33 AM, Thomas Stüfe wrote: > > > The established (since decades) method of forking off on Linux JDKs > > has been to use vfork(3). Using vfork is risky. There are chances of > > crashing the

Re: [11u]: RFR CSR backport: JDK-8220362: Add "POSIX_SPAWN" as valid value to jdk.lang.Process.launchMechanism on Linux

2019-03-19 Thread Thomas Stüfe
Hi Andrew, This would be good to have in jdk11 for the following reason: The established (since decades) method of forking off on Linux JDKs has been to use vfork(3). Using vfork is risky. There are chances of crashing the parent process. The risk of this happening is very low, but not zero.

Re: RFR(s): 8212828: (process) Change the Process launch mechanism default on Linux to be posix_spawn

2019-02-13 Thread Thomas Stüfe
to update the bug; wrong > bug, wrong updates. > > Thanks, Roger > > > On 2/12/19 1:41 AM, Thomas Stüfe wrote: > > > > On Mon, Feb 11, 2019 at 9:18 PM Martin Buchholz > wrote: > >> Looks good to me. >> >> > Thank you, I just pushed. > >

Re: RFR(s): 8212828: (process) Change the Process launch mechanism default on Linux to be posix_spawn

2019-02-12 Thread Thomas Stüfe
-if-a-test-does-not-apply-in-a-given-situation> > > http://openjdk.java.net/jtreg/faq.html#what-if-a-test-does-not-apply-in-a-given-situation > > Testng is generally preferred for new test over main tests and it has its > own Skipped test mechanism. > > Regards, Roger >

Re: RFR(s): 8212828: (process) Change the Process launch mechanism default on Linux to be posix_spawn

2019-02-11 Thread Thomas Stüfe
Rogers ProcessHandle API (the child seeking), but we still would need to find out if the child is a zombie. I like the test name btw. Very succinct :) ..Thomas > > On Mon, Feb 11, 2019 at 10:50 AM Thomas Stüfe > wrote: > >> Hi Roger, Martin, >> >> hopefully final ve

Re: RFR(s): 8212828: (process) Change the Process launch mechanism default on Linux to be posix_spawn

2019-02-11 Thread Thomas Stüfe
Thank you Roger! On Mon, Feb 11, 2019 at 7:55 PM Roger Riggs wrote: > Great! Looks fine. > > Thanks, Roger > > On 02/11/2019 01:50 PM, Thomas Stüfe wrote: > > Hi Roger, Martin, > > hopefully final version: > > > http://cr.openjdk.java.net/~stuefe/webrevs/8

  1   2   3   >