RFR: 8156153: java/lang/System/LoggerFinder/jdk/DefaultLoggerBridgeTest/DefaultLoggerBridgeTest.java fails with java.lang.RuntimeException

2016-05-06 Thread Daniel Fuchs
Hi, This is a fix for a subtle test bug caused by a logger being GCed. bug id: https://bugs.openjdk.java.net/browse/JDK-8156153 webrev: http://cr.openjdk.java.net/~dfuchs/webrev_8156153/webrev/ This test has been seen failing in hs integration with the following error:

Re: RFR: 8139982 Re-examine java.management dependency on java.util.logging.LoggingMXBean

2016-05-02 Thread Daniel Fuchs
On 02/05/16 20:06, Mandy Chung wrote: On May 2, 2016, at 10:59 AM, Daniel Fuchs <daniel.fu...@oracle.com> wrote: Hi Mandy, I applied the suggested changes. http://cr.openjdk.java.net/~dfuchs/8139982_webrev/webrev.08/index.html Looks very good and much cleaner. Nits:

Re: RFR: 8139982 Re-examine java.management dependency on java.util.logging.LoggingMXBean

2016-05-02 Thread Daniel Fuchs
Hi Mandy, I applied the suggested changes. http://cr.openjdk.java.net/~dfuchs/8139982_webrev/webrev.08/index.html best regards, -- daniel On 02/05/16 19:00, Mandy Chung wrote: hanks a lot for the feedback! > > New webrev here: >

Re: RFR: 8139982 Re-examine java.management dependency on java.util.logging.LoggingMXBean

2016-05-02 Thread Daniel Fuchs
Hi Mandy, Answers inline, and new webrev at the end. On 29/04/16 21:55, Mandy Chung wrote: Hi Daniel, On Apr 29, 2016, at 8:08 AM, Daniel Fuchs <daniel.fu...@oracle.com> wrote: Hi, Please find below a patch [2] that eliminates a static dependency of java.lang.mana

Re: RFR (JAXP) 8152912: SAX XMLReaderFactory needs to be ServiceLoader compliant

2016-05-09 Thread Daniel Fuchs
:22, huizhe wang wrote: On 5/3/2016 3:36 AM, Daniel Fuchs wrote: Hi Joe, This look good but the implementation might be overly complex, which makes it difficult to read. It was basically the existing code with some cleanup. What's in jarLookup was a copy of the original code. As you can see I

Re: RFR [9] 8153158: Remove sun.misc.ManagedLocalsThread from java.logging

2016-04-18 Thread Daniel Fuchs
On 18/04/16 08:01, Chris Hegarty wrote: 8056152 added a new constructor to java.lang.Thread to constructing Threads that do not inherit inheritable-thread-local initial values. Given there is now a supported API for creating such threads, other areas of the JDK should be updated to use it.

Re: RFR [9] 8147553: Remove sun.misc.ManagedLocalsThread from java.management

2016-04-18 Thread Daniel Fuchs
Hi Chris, The changes look good to me. best regards, -- daniel On 18/04/16 12:37, Chris Hegarty wrote: 8056152 added a new constructor to java.lang.Thread to constructing Threads that do not inherit inheritable-thread-local initial values. Given there is now a supported API for creating

Re: JDK 9 pre-review of JDK-6850612: Deprecate Class.newInstance since it violates the checked exception language contract

2016-04-27 Thread Daniel Fuchs
Hi Joe, Changes in java.util.logging and java.management look good. I glanced at the rest and spotted one issue here: http://cr.openjdk.java.net/~darcy/6850612.0/src/java.desktop/share/classes/javax/swing/text/html/HTMLEditorKit.java.frames.html 615 Object o =

Re: JAXP: XSLTC transformer swallows empty namespace declaration which is necessary to undeclare default namespace

2016-07-26 Thread Daniel Fuchs
Hi Christoph, I was at first a bit suspicious of your proposed patch, but I applied it and played with it and could not fault it. Now I tend to believe this is the correct thing to do (though that's not my area of expertise). I tried it with a bit more elaborate content in the "transform"

Re: RFR (JAXP) JDK-8067170: Enable security manager on JAXP unit tests

2016-07-25 Thread Daniel Fuchs
On 26/07/16 00:53, huizhe wang wrote: To avoid having to grant the permission, the test may choose to read the property before setting the SecurityManager. You might not be able to use TestNG Listeners in such case, or maybe you can by initializing the properties before the test starts. Or

Re: RFR (JAXP) JDK-8067170: Enable security manager on JAXP unit tests

2016-07-26 Thread Daniel Fuchs
On 26/07/16 04:24, Frank Yuan wrote: Thank you very much for your suggestions! Now I fully understand the rule(at least I think so :P) I will use a runWithAllPerm block surrounding the user setup code as Daniel's way. Btw, Daniel, ThreadLocal should not need Atomic any more, correct? Hi

Re: Review request: JDK-8160398 (jdeps) Replace a list of JDK 8 internal API for detecting if it's removed in JDK 9 or later

2016-07-13 Thread Daniel Fuchs
On 13/07/16 11:48, Mandy Chung wrote: On Jul 13, 2016, at 4:55 PM, Daniel Fuchs <daniel.fu...@oracle.com> wrote: > > Hi Mandy, > > I have not finished reviewing yet - but as early feedback I believe > that the following probably has a mistake: > > jdk.jdeps/share/

Re: --dry-run description enhancement

2016-07-12 Thread Daniel Fuchs
Hi Paul, On 7/12/16 8:00 AM, Paul Benedict wrote: Mandy, perhaps there is a JVM technicality here I'm unfamiliar with. My understanding of loading a class has always been that it's coupled with running its static initializers. So my inference was that the class does not get loaded into memory

[JAXP] RFR: 8153082: Update XSTL compiler to generate classes that invoke addReads

2016-07-22 Thread Daniel Fuchs
Hi, Please find below a fix for 8153082: Update XSTL compiler to generate classes that invoke addReads https://bugs.openjdk.java.net/browse/JDK-8153082 This fix removes a dependency from java.xml to an internal java base API. http://cr.openjdk.java.net/~dfuchs/webrev_8153082/webrev.00/ It

Re: [JAXP] RFR: 8153082: Update XSTL compiler to generate classes that invoke addReads

2016-07-25 Thread Daniel Fuchs
Hi Christoph, On 22/07/16 20:23, Langer, Christoph wrote: Hi Daniel, looks good to me. Maybe you'll want to take the chance to update the apache headers in the xalan files? Thanks for your review! I'm only an occasional wanderer in JAXP land - which files do you see have an outdated

Re: [JAXP] RFR: 8153082: Update XSTL compiler to generate classes that invoke addReads

2016-07-25 Thread Daniel Fuchs
Hi Alan, On 23/07/16 07:57, Alan Bateman wrote: On 22/07/2016 17:16, Daniel Fuchs wrote: Hi, Please find below a fix for 8153082: Update XSTL compiler to generate classes that invoke addReads https://bugs.openjdk.java.net/browse/JDK-8153082 This fix removes a dependency from java.xml

Re: [JAXP] RFR: 8153082: Update XSTL compiler to generate classes that invoke addReads

2016-07-25 Thread Daniel Fuchs
was unsure whether to keep it or remove it. I will remove it before pushing. best regards, -- daniel Best regards Christoph -Original Message- From: Daniel Fuchs [mailto:daniel.fu...@oracle.com] Sent: Montag, 25. Juli 2016 19:22 To: Langer, Christoph <christoph.lan...@sap.com>; Jo

Re: [JAXP] RFR: 8153082: Update XSTL compiler to generate classes that invoke addReads

2016-07-25 Thread Daniel Fuchs
. It is a good idea to consult Joe on this - he was also giving me hints on how to do it correctly when I was touching JAXP. Best regards Christoph -Original Message- From: Daniel Fuchs [mailto:daniel.fu...@oracle.com] Sent: Montag, 25. Juli 2016 16:43 To: Langer, Christoph <christoph.

Re: RFR [9] 8160513: ClassNotFoundException sun.misc.GC when running Tomcat 9 with JDK 9

2016-07-27 Thread Daniel Fuchs
On 26/07/16 16:24, Chris Hegarty wrote: The GC.Daemon thread has no need of any user defined class loader, so should set its context class loader to null before starting, so as to not inadvertently retain a reference to the creating thread’s context class loader.

Re: RFR [9] 8157570: sun.rmi.transport.GC retains a strong reference to the context class loader ( was :8160513 )

2016-07-28 Thread Daniel Fuchs
Hi Chris, Looks good to me! best regards, -- daniel On 28/07/16 12:40, Chris Hegarty wrote: On 28 Jul 2016, at 12:28, Alan Bateman wrote: On 28/07/2016 11:22, Chris Hegarty wrote: [ switching to 8157570 as it better describes the issue, rather than the affect ]

Re: RFR (JAXP): 8162598 XSLTC transformer swallows empty namespace declaration which is needed to undeclare default namespace

2016-07-28 Thread Daniel Fuchs
Hi Christoph, Looks good in general, even though the idiom if (existing instanceof Stack) caught my eye. Thanks for the new test! I wonder if it should be made more strict - with a golden record of the expected results. In particular, to check that the xmlns="" in element is removed only

Re: RFR [9] 8156824: com.sun.jndi.ldap.pool.PoolCleaner should clear its context class loader

2016-07-28 Thread Daniel Fuchs
On 28/07/16 14:22, Chris Hegarty wrote: Another issue where an internal platform thread is unnecessarily retaining a reference to its creating thread’s context class loader. In this case, it appears to be safe to use an InnocuousThread, which will have a null context class loader.

Re: RFR [9] 8156824: com.sun.jndi.ldap.pool.PoolCleaner should clear its context class loader

2016-07-28 Thread Daniel Fuchs
On 28/07/16 15:11, Chris Hegarty wrote: On 28 Jul 2016, at 14:52, Daniel Fuchs <daniel.fu...@oracle.com> wrote: > > On 28/07/16 14:22, Chris Hegarty wrote: >> Another issue where an internal platform thread is unnecessarily >> retaining a reference to its creating threa

Re: RFR (JAXP): 8162598 XSLTC transformer swallows empty namespace declaration which is needed to undeclare default namespace

2016-07-28 Thread Daniel Fuchs
Hi Christoph, On 28/07/16 16:05, Langer, Christoph wrote: Looks good in general, even though the idiom >if (existing instanceof Stack) > caught my eye. I didn't like it either but found no better way to get rid of the warnings. If you have a better idea here, let me know :) The

Re: RFR (JAXP) JDK-8067170: Enable security manager on JAXP unit tests

2016-07-22 Thread Daniel Fuchs
Hi Frank, I see that in order to be able to run the tests, you were forced to add a few permissions that the test/test infrastructure need to setup things: 107 addPermission(new SecurityPermission("getPolicy")); 108 addPermission(new SecurityPermission("setPolicy")); 109

Re: RFR: 6543126: Level.known can leak memory

2016-07-28 Thread Daniel Fuchs
On 28/07/16 21:31, James Perkins wrote: I just happened to take a glance at this and noticed the remove method on the KnownLevels doesn't quite look right. Using Optional.of(nameToLevels.get(mirroredLevel.name)).ifPresent((x) -> x.remove(this)); will produce an NPE if the level is not present.

Re: RFR: 6543126: Level.known can leak memory

2016-07-29 Thread Daniel Fuchs
Hi, Here is the new webrev with Chris & James feedback taken in. http://cr.openjdk.java.net/~dfuchs/webrev_6543126/webrev.01/ best regards, -- daniel On 28/07/16 22:55, Daniel Fuchs wrote: On 28/07/16 21:31, James Perkins wrote: I just happened to take a glance at this and not

Re: [JAXP] RFR: 8153082: Update XSTL compiler to generate classes that invoke addReads

2016-07-26 Thread Daniel Fuchs
On 26/07/16 10:44, Alan Bateman wrote: On 25/07/2016 18:21, Daniel Fuchs wrote: Hi, Here is the later version of the fix: - Header files fixed - Bytecode 1.1 compatible http://cr.openjdk.java.net/~dfuchs/webrev_8153082/webrev.02/ This looks okay to me although I think I'd like

Re: JAXP: XSLTC transformer swallows empty namespace declaration which is necessary to undeclare default namespace

2016-07-26 Thread Daniel Fuchs
Hi Christoph, On 26/07/16 14:53, Langer, Christoph wrote: Hi Daniel, you mean with my webrev built in or with the current jdk9 dev? With the current jdk9 dev - sorry if that was unclear. cheers, -- daniel public static void main(String[] args) throws XMLStreamException,

Re: JAXP: XSLTC transformer swallows empty namespace declaration which is necessary to undeclare default namespace

2016-07-26 Thread Daniel Fuchs
Hi Christoph, Joe, Actually what I see with the latest dev version of JDK 9 (eng. build built a few minutes ago) is this: xmlns="ns1"> Notice that xmlns="ns1" appears twice in the root element. So maybe there's more than one bug here :-( cheers, -- daniel On 26/07/16 12:56, Langer,

Re: JAXP: XSLTC transformer swallows empty namespace declaration which is necessary to undeclare default namespace

2016-07-26 Thread Daniel Fuchs
On 26/07/16 14:53, Langer, Christoph wrote: you mean with my webrev built in or with the current jdk9 dev? ... and with your patch applied I see this: xmlns="ns1"> cheers, -- daniel

Re: RFR (JAXP) JDK-8067170: Enable security manager on JAXP unit tests

2016-07-27 Thread Daniel Fuchs
r = transFactory.newTransformer(); @@ -111,7 +115,7 @@ + " Hello World!\n" + "\n"; ReaderStub.used = false; -System.setProperty("org.xml.sax.driver", ReaderStub.class.getName()); +setSystemProperty("org.xml.sax.driver", Reade

Re: RFR: 6543126: Level.known can leak memory

2016-07-27 Thread Daniel Fuchs
Hi Chris, On 27/07/16 11:17, Chris Hegarty wrote: Hi Daniel, On 25/07/16 19:10, Daniel Fuchs wrote: Hi, Please find below a fix for: 6543126: Level.known can leak memory https://bugs.openjdk.java.net/browse/JDK-6543126 webrev: http://cr.openjdk.java.net/~dfuchs/webrev_6543126/webrev.00

Re: RFR 9: JEP 290: Filter Incoming Serialization Data

2016-07-27 Thread Daniel Fuchs
Hi Roger, ObjectInputStream.java: 179 * If a {@link #setObjectInputFilter(ObjectInputFilter) filter is set} 184 * A {@link ObjectInputFilter.Config#setSerialFilter(ObjectInputFilter) process-wide filter} these two lines should be using {@linkplain, not {@link. 308 private

Re: RFR (JAXP) JDK-8067170: Enable security manager on JAXP unit tests

2016-08-02 Thread Daniel Fuchs
Hi Frank, I am intrigued by these change which do not seem to have anything to do with the rest. I mean, I'm not opposed as long as they are intended and that Joe validates them:

Re: RFR 9: [TESTBUG] 8160151: java/lang/ProcessBuilder/Zombies.java failed with error "1 zombies!"

2016-08-02 Thread Daniel Fuchs
Hi Roger, Running in othervm looks good to me. The only thing I wonder about is these lines which are removed: 79 // Log remaining processes 80 ProcessBuilder pb = new ProcessBuilder("/bin/ps", "-ef"); 81 pb.inheritIO(); 82 Process p2 =

Re: RFR 9: [TESTBUG] 8160151: java/lang/ProcessBuilder/Zombies.java failed with error "1 zombies!"

2016-08-02 Thread Daniel Fuchs
. (There is no harm in retaining this extra info but I didn't think it was particularly useful.) Thanks Roger. No issue removing this code then. +1 -- daniel Thanks, Roger On 8/2/2016 9:59 AM, Daniel Fuchs wrote: Hi Roger, Running in othervm looks good to me. The only thing I wonder about

Re: [jdk9] RFR (XXS): 8163180: Typos around @code javadoc tag usage

2016-08-04 Thread Daniel Fuchs
Hi Ivan, Looks good! best regards, -- daniel On 04/08/16 13:52, Ivan Gerasimov wrote: Hello! In a few places the @code javadoc tag misses the curly bracket. Would you please review a trivial fix? http://cr.openjdk.java.net/~igerasim/8163180/00/webrev/ With kind regards, Ivan

Re: RFR: 6543126: Level.known can leak memory

2016-08-11 Thread Daniel Fuchs
On 10/08/16 17:21, Mandy Chung wrote: On Jul 29, 2016, at 4:54 AM, Daniel Fuchs <daniel.fu...@oracle.com> wrote: > > http://cr.openjdk.java.net/~dfuchs/webrev_6543126/webrev.01/ This looks pretty good. Since KnownLevel is now a Reference, I suggest to change KnownLevel

Re: RFR: 6543126: Level.known can leak memory

2016-08-11 Thread Daniel Fuchs
Hi Peter, On 10/08/16 22:06, Peter Levart wrote: static synchronized void add(Level l) { purge(); KnownLevel o = new KnownLevel(l); nameToLevels.computeIfAbsent(l.name, n -> new ArrayList<>()).add(o); intToLevels.computeIfAbsent(l.value, v -> new ArrayList<>()).add(o); } I

Re: RFR (JAXP) JDK-8163468: javax/xml/jaxp/unittest/validation/Bug6773084Test.java fails intermittently

2016-08-10 Thread Daniel Fuchs
Hi Frank, Good analysis of the failure root cause! The proposed fix looks good to me. As a side note, are there other multi-threaded tests in JAXP? If so maybe you'll need a special method in JAXPSecurityManager to transfer the permissions of the current to another thread (I mean - find a way

Re: Review Request: JDK-8157464: StackWalker.getCallerClass() should not filter out non-invoker frames

2016-07-14 Thread Daniel Fuchs
special to do here since we already checked + // StackWalk::get_caller_class(mode) above } if (++frames_decoded >= max_nframes) break; } return frames_decoded; } Mandy On Jul 13, 2016, at 10:55 PM, Daniel Fuchs <daniel.fu...@oracle.com> wrote: Hi Mandy, I wonder whe

Re: Review Request: JDK-8157464: StackWalker.getCallerClass() should not filter out non-invoker frames

2016-07-13 Thread Daniel Fuchs
Hi Mandy, I wonder whether intermediate frames should be skipped always, whether the method is @CS or not. Indeed StackWalker::getCallerClass() is intented to be called from methods that are not @CS. If so the code in stackwalk.cpp could probably be simplified to simply look at

Re: RFR(XS): 8160457: VersionProps.versionNumbers() is broken

2016-06-28 Thread Daniel Fuchs
Hi, WRT to testing - one thing that could be done (possibly in a followup patch) would be: class VersionProps { ... static List parseVersionNumbers(String versionNumber) { // parsing code goes there } static List versionNumbers() { return

RFR: 8056285: java/util/logging/CheckLockLocationTest.java java.lang.RuntimeException: Test failed: should have been able to create FileHandler for %t/writable-dir/log.log in writable directory.

2016-06-28 Thread Daniel Fuchs
Hi, JDK-8056285 has been sighted again (sigh). I strongly suspect a configuration issue but I have no proof. Here is an attempt at gathering some more data to nail down the root cause of the issue. JBS https://bugs.openjdk.java.net/browse/JDK-8056285 webrev:

Re: [9] RFR 8160611: Clean up ProblemList.txt for closed/resolved issues

2016-08-09 Thread Daniel Fuchs
Hi John, JDK-8061177 [1] has been resolved as a duplicate of JDK-8065756 [2] which is still open. The correct action for this one might be to leave the test in the problem list but change the bug ID. The rest looks good to me - even though two of these test have been fixed by either adding

Re: RFR (JAXP) JDK-8067170: Enable security manager on JAXP unit tests

2016-07-22 Thread Daniel Fuchs
On 22/07/16 10:15, Frank Yuan wrote: Hi Daniel Thank you very much for your review and the comments! -Original Message- From: Daniel Fuchs [mailto:daniel.fu...@oracle.com] Subject: Re: RFR (JAXP) JDK-8067170: Enable security manager on JAXP unit tests Hi Frank, I see that in order

Re: RFR: jsr166 jdk9 integration wave 7

2016-06-30 Thread Daniel Fuchs
On 30/06/16 01:04, Doug Lea wrote: On Wed, Jun 29, 2016 at 8:38 AM, Daniel Fuchs <daniel.fu...@oracle.com <mailto:daniel.fu...@oracle.com>> wrote: I mean something like: "The maximum number of spare threads used by the common pool is 256: to arrange the same value as i

Re: RFR 8159245: Loggers created by system classes are not initialized correctly when configured programmatically from application code.

2016-06-30 Thread Daniel Fuchs
Hi Mandy, On 21/06/16 17:01, Mandy Chung wrote: On Jun 21, 2016, at 8:39 AM, Daniel Fuchs <daniel.fu...@oracle.com> wrote: I don't understand this scenario. ConfigurationData should remain as simple as possible. Logger.getLogger() / LogManager.demandSystemLogger() will never return a

Re: RFR: jsr166 jdk9 integration wave 7

2016-06-29 Thread Daniel Fuchs
Hi Martin, I was looking at the new constructor's API documentation in ForkJoinPool - and somehow got confused by the sentence: 2235 * @param maximumPoolSize [... 2241 * ...] To 2240 * arrange the same value as is used by default for the common 2241 * pool, use {@code 256}

Re: RFR 8159245: Loggers created by system classes are not initialized correctly when configured programmatically from application code.

2016-07-01 Thread Daniel Fuchs
On 01/07/16 16:33, Mandy Chung wrote: I think the additional check is unecessary but it’s harmless if you prefre to keep that. Oh - I see - the permission check in ConfigurationData::merge can be removed: ConfigurationData is a private class and config is a private field. I will do that.

Re: RFR 8159245: Loggers created by system classes are not initialized correctly when configured programmatically from application code.

2016-07-01 Thread Daniel Fuchs
On 01/07/16 16:19, Mandy Chung wrote: I'd prefer to keep the doPrivileged in LogManager so that > Logger.mergeWithSystemLogger can call checkpermission(). > > From a conceptual point of view it's only when calling > this method from LogManager that we want to be privileged, > even though the

Re: RFR: 6543126: Level.known can leak memory

2016-08-16 Thread Daniel Fuchs
wrote: On Aug 11, 2016, at 2:29 AM, Daniel Fuchs <daniel.fu...@oracle.com> wrote: On 10/08/16 17:21, Mandy Chung wrote: On Jul 29, 2016, at 4:54 AM, Daniel Fuchs <daniel.fu...@oracle.com> wrote: http://cr.openjdk.java.net/~dfuchs/webrev_6543126/webrev.01/ This looks pretty

Re: RFR: 6543126: Level.known can leak memory

2016-08-16 Thread Daniel Fuchs
Regards, Peter On 08/16/2016 12:42 PM, Daniel Fuchs wrote: Hi Mandy, I added an additional selector parameter to the find methods. This made it possible to return Optional instead of KnownLevel - and it does simply the parse() method. http://cr.openjdk.java.net/~dfuchs/webrev_6543126/webrev.02

Re: RFR: 6543126: Level.known can leak memory

2016-08-17 Thread Daniel Fuchs
Hi Peter, Rereading the comment and looking at the code I see that the comment is actually right. There are two methods with similar names: Level.getLocalizedName() which can be overridden by subclasses and Level.getLocalizedLevelName() which cannot. By default Level.getLocalizedName() calls

Re: RFR: 6543126: Level.known can leak memory

2016-08-17 Thread Daniel Fuchs
rev.02 best regards, -- daniel On 11/08/16 20:12, Mandy Chung wrote: On Aug 11, 2016, at 2:29 AM, Daniel Fuchs <daniel.fu...@oracle.com> wrote: On 10/08/16 17:21, Mandy Chung wrote: On Jul 29, 2016, at 4:54 AM, Daniel Fuchs <daniel.fu...@oracle.com> wrote: http://cr.openjdk.jav

Re: RFR: 6543126: Level.known can leak memory

2016-08-17 Thread Daniel Fuchs
ClassLoader with a strong reference. You could use a ClassLoaderValue for this purpose, like in the following addition to your patch (see KnownLevel.add): http://cr.openjdk.java.net/~plevart/jdk9-dev/Level.known/webrev.01/ Regards, Peter On 08/16/2016 12:42 PM, Daniel Fuchs wrote: Hi Mandy, I

RFR: 8173898: StackWalker.walk throws InternalError if called from a constructor invoked through reflection.

2017-02-03 Thread Daniel Fuchs
Hi, Please find below a simple fix for: 8173898: StackWalker.walk throws InternalError if called from a constructor invoked through reflection. https://bugs.openjdk.java.net/browse/JDK-8173898 http://cr.openjdk.java.net/~dfuchs/webrev_8173898/webrev.00/ best regards, -- daniel

Re: RFR: 8173898: StackWalker.walk throws InternalError if called from a constructor invoked through reflection.

2017-02-03 Thread Daniel Fuchs
Hi, Please find below a new webrev. Part of the fix was mising in the previous one. I also took the opportunity to replace the test's assertEquals with those from org.testng.Assert. http://cr.openjdk.java.net/~dfuchs/webrev_8173898/webrev.01 best regards, -- daniel On 03/02/17 19:51, Daniel

Re: [RFR] 8160768: Add capability to custom resolve host/domain names within the default JDNI LDAP provider

2017-02-02 Thread Daniel Fuchs
/cr.openjdk.java.net/~robm/8160768/webrev.02/ I'll add a couple of tests for the SecurityManager along with some input checking tests too. -Rob On 25/01/17 05:50, Daniel Fuchs wrote: Hi Rob, I believe you should move the security check to before the class is actually loaded, before the

Re: RFR: 8173898: StackWalker.walk throws InternalError if called from a constructor invoked through reflection.

2017-02-03 Thread Daniel Fuchs
Hi Paul, On 03/02/17 21:54, Paul Sandoz wrote: On 3 Feb 2017, at 11:51, Daniel Fuchs <daniel.fu...@oracle.com> wrote: Hi, Please find below a simple fix for: 8173898: StackWalker.walk throws InternalError if called from a constructor invoked through reflection.

RFR: 8173607: JMX RMI connector should be in its own module

2017-01-31 Thread Daniel Fuchs
Hi, Please find below a patch for: 8173607: JMX RMI connector should be in its own module https://bugs.openjdk.java.net/browse/JDK-8173607 webrev: http://cr.openjdk.java.net/~dfuchs/webrev_8173607/webrev.05 This patch makes it possible to remove the requires transitive java.rmi from the

Re: RFR: 8173607: JMX RMI connector should be in its own module

2017-02-01 Thread Daniel Fuchs
Hi Mandy, On 01/02/17 05:11, Mandy Chung wrote: On Jan 31, 2017, at 10:26 AM, Daniel Fuchs <daniel.fu...@oracle.com> wrote: Hi, Please find below a patch for: 8173607: JMX RMI connector should be in its own module https://bugs.openjdk.java.net/browse/JDK-8173607 webrev

Re: RFR: 8173898: StackWalker.walk throws InternalError if called from a constructor invoked through reflection.

2017-02-07 Thread Daniel Fuchs
::getCallerClass: http://cr.openjdk.java.net/~dfuchs/webrev_8173898/webrev.03/StackWalker-report.html best regards, -- daniel Mandy On Feb 3, 2017, at 11:51 AM, Daniel Fuchs <daniel.fu...@oracle.com> wrote: Hi, Please find below a simple fix for: 8173898: StackWalker.walk throws Interna

Re: RFR: 8173898: StackWalker.walk throws InternalError if called from a constructor invoked through reflection.

2017-02-08 Thread Daniel Fuchs
On 08/02/17 15:58, Mandy Chung wrote: I disagree: the text starts by saying: "By default reflection frames are hidden. The reflection frames include […]" 209 * Shows all reflection frames. This is the first sentence. line 216-217 repeats line 209. but the sentence at lines 216-217

Re: RFR: 8173607: JMX RMI connector should be in its own module

2017-02-02 Thread Daniel Fuchs
Hi Mandy, On 02/02/17 02:41, Mandy Chung wrote: http://cr.openjdk.java.net/~dfuchs/webrev_8173607/webrev.06 Does java.management still depend on java.base/jdk.internal.module? Well spotted! It doesn't. I have updated module-info.java for java.base.

Re: JDK 9: 8174128: [testbug] Remove implementation dependency from java.time TCK tests

2017-02-08 Thread Daniel Fuchs
Looks good! -- daniel On 07/02/17 20:06, Roger Riggs wrote: Please review minor changes to the java.time.tck tests to avoid the use of implementation details and the related opening of the modules. The testng directive to open needed modules is now specific to the java.time.test... tests.

Re: RFR: 8173898: StackWalker.walk throws InternalError if called from a constructor invoked through reflection.

2017-02-08 Thread Daniel Fuchs
Hi Mandy, Thanks for all the suggestions! On 07/02/17 20:03, Mandy Chung wrote: Please find below a new webrev that incorporates your feedback. http://cr.openjdk.java.net/~dfuchs/webrev_8173898/webrev.03/ I think this sentence is not needed. 216 * A {@code StackWalker} with this

Re: RFR(s): 8174194: Several java/lang tests failing due to undeclared module dependencies.

2017-02-08 Thread Daniel Fuchs
Hi Sergey, LFMultiThreadCachingTest.java Can you clarify the change in this file? jdk.management requires java.management - so if the test failed with --limit-modules when it required jdk.management then it's not going to pass if it is changed to require only java.management. Or is it that

Re: RFR(s): 8174194: Several java/lang tests failing due to undeclared module dependencies.

2017-02-08 Thread Daniel Fuchs
On 08/02/17 14:13, Sergei Kovalev wrote: Actually jdk.management is not required for this particular test. java.management module is enough to pass the test. Well, looks good to me then! best regards, -- daniel

Re: [RFR] 8160768: Add capability to custom resolve host/domain names within the default JDNI LDAP provider

2017-02-03 Thread Daniel Fuchs
. I see that Vyom has taken a look at your patch and suggested a change too and that is good :-) best regards, -- daniel -Rob On 02/02/17 11:01, Daniel Fuchs wrote: Hi Rob, This is not a code I know well - but here are a couple of comments: LdapCtxFactory.java: 168

Re: Ping: [JAXP] RFR: 8173602: TESTBUG: javax/xml/jaxp/unittest/transform/TransformerTest.java needs refactoring

2017-02-03 Thread Daniel Fuchs
Hi Christoph, I had a look at your patch, it seems OK. I didn't know that testng supported @Test annotations on methods of inner classes, but that looks useful. The patch is a bit difficult to review because the diff seems a bit lost sometime - but as far as I could see what was there before

Re: Ping: [JAXP] RFR: 8173602: TESTBUG: javax/xml/jaxp/unittest/transform/TransformerTest.java needs refactoring

2017-02-03 Thread Daniel Fuchs
. Best regards Christoph -Original Message- From: Daniel Fuchs [mailto:daniel.fu...@oracle.com] Sent: Freitag, 3. Februar 2017 13:54 To: Langer, Christoph <christoph.lan...@sap.com>; core-libs- d...@openjdk.java.net Subject: Re: Ping: [JAXP] RFR: 8173602: TESTBUG: javax/xml/jaxp/un

Heads up: Fwd: hg: jdk9/dev/jdk: 8173607: JMX RMI connector should be in its own module

2017-02-02 Thread Daniel Fuchs
Hi, A direct consequence of this change is that you may have to either blow up your ./build directory, or run 'make clean-java' after pulling this fix. Otherwise you may see strange build failures like below: Error occurred during initialization of VM java.lang.RuntimeException: Package

Re: RFR 8173159, Problem list java/rmi/activation/ActivationGroup/downloadActivationGroup/DownloadActivationGroup.java on Windows

2017-01-21 Thread Daniel Fuchs
+1 -- daniel On 21/01/17 05:43, Felix Yang wrote: Hi there, please review the request to problem-list the test on Windows platform. It has been observed to be failing frequently. Bug https://bugs.openjdk.java.net/browse/JDK-8173159

Re: RFR (JAXP) 8169827: javax/xml/jaxp/isolatedjdk/catalog/PropertiesTest.sh copied JDK failed

2017-01-23 Thread Daniel Fuchs
Hi Christoph, Thanks for fixing this test! I imported your patch, modified ProblemList.txt to not skip the test, and sent it through our test system, and I'm happy to report that the test was run on all available platforms with no failure. So I think you should simply remove the line from

Re: (JAXP) RFR: 8173111: Excessive recursion in EventFilterSupport when filtering over large number of XML events can cause StackOverflow

2017-01-24 Thread Daniel Fuchs
67. I kept the main method (useful when you tweak the test in NetBeans). I'm going to launch it again through our test system and if successful I'll consider it reviewed and push it, unless I hear otherwise by then :-) best regards, -- daniel On 23/01/17 17:48, Daniel Fuchs wrote: Hi, Please

[JAXP] RFR: 8173260: CatalogManager.catalogResolver should not fail when non-existing URI is passed to it

2017-01-26 Thread Daniel Fuchs
Hi, Please find below a fix for 8173260: CatalogManager.catalogResolver should not fail when non-existing URI is passed to it https://bugs.openjdk.java.net/browse/JDK-8173260 The specification for CatalogManager.catalogResolver and CatalogManager.catalog says that invalid catalog

Re: RFR 8170116: Remove qualified exports from java.base to java.corba

2017-01-25 Thread Daniel Fuchs
On 25/01/17 12:32, Alan Bateman wrote: On 25/01/2017 11:05, Pavel Rappo wrote: Hello, Could you please review the following change for [1]? http://cr.openjdk.java.net/~prappo/8170116/webrev.00/ Hi Alan, I agree with Daniel on the name. Also the comment in the Bridge constructor

Re: RFR 8170116: Remove qualified exports from java.base to java.corba

2017-01-25 Thread Daniel Fuchs
Hi Pavel, Looks good to me. I might have kept the 'defaultPresentationManager' name though. Even though it's global it feels like a better name than 'globalPM'. best regards, -- daniel On 25/01/17 11:05, Pavel Rappo wrote: Hello, Could you please review the following change for [1]?

Re: [RFR] 8160768: Add capability to custom resolve host/domain names within the default JDNI LDAP provider

2017-01-25 Thread Daniel Fuchs
Hi Rob, I believe you should move the security check to before the class is actually loaded, before the call to 171 List urls = getDnsUrls(url, env); best regards, -- daniel On 25/01/17 17:44, Rob McKenna wrote: I neglected to include a security check so I've cribbed the one

Re: Soften interface for javax.management.ObjectName.getInstance and friends

2017-02-24 Thread Daniel Fuchs
Hi Dave, I'm not sure this is quite a good idea because order doesn't count when comparing ObjectNames. So the folder analogy would just be a lure: /a/b/c would be equal to /a/c/b That said - I can see value in trying to get rid of the legacy Hashtable - so adding a new method that takes a

Re: RFR (JAXP) 8169450: StAX parse error if there is a newline in xml declaration

2017-02-14 Thread Daniel Fuchs
On 14/02/17 17:21, huizhe wang wrote: Thanks! Here's an updated webrev: http://cr.openjdk.java.net/~joehw/jdk9/8169450/webrev/ +1 -- daniel -Joe On 2/14/2017 4:07 AM, Lance Andersen wrote: Looks good overall Joe. I would agree that I would clean up the minor comment alignment issues.

Re: RFR (JAXP) 8169450: StAX parse error if there is a newline in xml declaration

2017-02-14 Thread Daniel Fuchs
Looks good Joe. I wonder about this though (which may be an issue for another time): 102 [25] Eq ::= S? '=' S? Do we support space (new line?) before and after the '=' sign? best regards, -- daniel On 14/02/17 02:27, huizhe wang wrote: A quick fix for the error parsing xml

Re: RFR [JAXP] JDK-8174025 Regression in XML Transform caused by JDK-8087303

2017-02-14 Thread Daniel Fuchs
e.w...@oracle.com] Subject: Re: RFR [JAXP] JDK-8170192 Regression in XML Transform caused by JDK-8087303 +1 from me too. Thanks, Joe On 2/10/2017 5:25 AM, Daniel Fuchs wrote: Hi Frank, Thanks for fixing this! I imported your patch and played with it a bit. Also ran the jaxp test. Both issues repor

Re: RFR [JAXP] JDK-8174025 Regression in XML Transform caused by JDK-8087303

2017-02-14 Thread Daniel Fuchs
Hi Frank, On 14/02/17 13:43, Frank Yuan wrote: -Original Message- From: Daniel Fuchs [mailto:daniel.fu...@oracle.com] Subject: Re: RFR [JAXP] JDK-8174025 Regression in XML Transform caused by JDK-8087303 Hi Frank, Should you skip '\r' if it's not followed by '\n'? Well - I'll let

Re: RFR [JAXP] JDK-8170192 Regression in XML Transform caused by JDK-8087303

2017-02-10 Thread Daniel Fuchs
Hi Frank, Thanks for fixing this! I imported your patch and played with it a bit. Also ran the jaxp test. Both issues reported have indeed disappeared. So that's a +1 from me. best regards, -- daniel On 10/02/17 11:03, Frank Yuan wrote: Hi All Would you like to review

Re: RFR: 8174735 Update JAX-WS RI integration to latest version

2017-02-16 Thread Daniel Fuchs
Hi Aleksej, On 15/02/17 23:49, Aleks Efimov wrote: Hi, The new webrev with addressed comments was uploaded here: http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8174735/01 This is probably a question for the upstream project, but I'm puzzled by this change:

Re: rmid on Unix fails with Exception - maybe aftermath of JDK-8173607 ??

2017-02-16 Thread Daniel Fuchs
Hi Christoph, It looks like one of the dreaded class initialization cycle issues. If you look at the stack trace, you will see that UnixNativeDispatcher.:609 calls System.loadLibrary at line 611 which later down the road calls the native UnixNativeDispatcher.getcwd command from

Re: rmid on Unix fails with Exception - maybe aftermath of JDK-8173607 ??

2017-02-16 Thread Daniel Fuchs
when that started showing up? best regards, -- daniel Best regards Christoph -Original Message- From: Daniel Fuchs [mailto:daniel.fu...@oracle.com] Sent: Donnerstag, 16. Februar 2017 15:53 To: Langer, Christoph <christoph.lan...@sap.com> Cc: Zeller, Arno <arno.zel...@sap.com&

Re: [9] RFR: 8173390: Investigate SymbolTable in SAXParser

2017-02-15 Thread Daniel Fuchs
Hi Aleksej, In the test: 75 Assert.assertNotEquals(symTable1, symTable2, "Symbol table refence is the same"); I guess you meant Assert.assertNotSame - if what you want to do is compare references. (+ there is a typo in the message: refence => reference) best regards, -- daniel

(JAXP) RFR: 8173111: Excessive recursion in EventFilterSupport when filtering over large number of XML events can cause StackOverflow

2017-01-23 Thread Daniel Fuchs
Hi, Please find below a fix for: 8173111: Excessive recursion in EventFilterSupport when filtering over large number of XML events can cause StackOverflow https://bugs.openjdk.java.net/browse/JDK-8173111 The fix is almost trivial: it replaces a recursion by a loop in two places.

Re: RFR 8172350: Typo in Timestamp.toString()

2017-01-19 Thread Daniel Fuchs
Hi Lance, On 19/01/17 15:20, Lance Andersen wrote: Hi all, Following trivial javadoc update to Timestamp.toString() needs a review Looks good! -- daniel — ljanders-mac:jdk ljanders$ hg diff diff -r 051e7d9159a7 src/java.sql/share/classes/java/sql/Timestamp.java ---

8172971: java.management could use System.Logger

2017-01-19 Thread Daniel Fuchs
Hi, Please find below a patch for: 8172971: java.management could use System.Logger https://bugs.openjdk.java.net/browse/JDK-8172971 webrev: http://cr.openjdk.java.net/~dfuchs/webrev_8172971/webrev.00/ I have also added a new test: test/sun/management/LoggingTest/LoggingTest.java This is a

Re: [9] RFR: 8171139: Simplify ResourceBundle.CacheKey and ClassLoader may not be needed

2017-01-17 Thread Daniel Fuchs
one, and the latter one for 8171140 was modified following that change. Naoto On 1/17/17 4:24 AM, Daniel Fuchs wrote: +1 With Peter's proposed changes if I'm not mistaken then all methods that operate on the CacheKey (findBundle, findBundleInCache, putBundleInCache, loadBundle, loadBundleFromProv

Re: [9] RFR: 8171139: Simplify ResourceBundle.CacheKey and ClassLoader may not be needed

2017-01-16 Thread Daniel Fuchs
Hi Naoto, On 14/01/17 00:54, Naoto Sato wrote: diff -r a6d3c80ea436 src/java.base/share/classes/java/util/ResourceBundle.java --- a/src/java.base/share/classes/java/util/ResourceBundle.java +++ b/src/java.base/share/classes/java/util/ResourceBundle.java @@ -2192,7 +2192,7 @@ private static

Re: [9] RFR: 8171139: Simplify ResourceBundle.CacheKey and ClassLoader may not be needed

2017-01-17 Thread Daniel Fuchs
t. Here's those two changes applied to your webrev.05 and also a race in clearCacheImpl() fixed (as reported by Daniel Fuchs): http://cr.openjdk.java.net/~plevart/jdk9-dev/8170772_ResourceBundle.caching/webrev.06/ What do you think? Regards, Peter

RFR: 8172886: Add a test that shows how the LogManager can be implemented by a module

2017-01-17 Thread Daniel Fuchs
Hi, Please find below a patch that adds a test that verifies that LogManager, Handlers, and config classes can be implemented from within a module (provided they are exported to java.logging). JBS: https://bugs.openjdk.java.net/browse/JDK-8172886 webrev:

Re: 8172971: java.management could use System.Logger

2017-01-19 Thread Daniel Fuchs
more difficult to review. best regards, -- daniel Roger On 1/19/2017 12:12 PM, Mandy Chung wrote: On Jan 19, 2017, at 7:30 AM, Daniel Fuchs <daniel.fu...@oracle.com> wrote: Hi, Please find below a patch for: 8172971: java.management could use System.Logger https://bugs.ope

Re: 8172971: java.management could use System.Logger

2017-01-19 Thread Daniel Fuchs
Hi Mandy, Thanks for the review! On 19/01/17 17:12, Mandy Chung wrote: On Jan 19, 2017, at 7:30 AM, Daniel Fuchs <daniel.fu...@oracle.com> wrote: Hi, Please find below a patch for: 8172971: java.management could use System.Logger https://bugs.openjdk.java.net/browse/JDK-8172971

<    2   3   4   5   6   7   8   9   10   11   >