Re: RFR 8170408: LogGeneratedClassesTest.java fails with recent changes

2016-11-29 Thread Daniel Fuchs
On 29/11/16 13:33, Wang Weijun wrote: http://cr.openjdk.java.net/~weijun/8170408/webrev.01 jdk_lang passes on all JPRT platforms. Looks good to me Max! best regards, -- daniel Thanks Max On Nov 29, 2016, at 9:26 PM, Daniel Fuchs <daniel.fu...@oracle.com> wrote: Hi Max, On 29/11

Re: RFR 8170408: LogGeneratedClassesTest.java fails with recent changes

2016-11-29 Thread Daniel Fuchs
unt(), Thanks Max On Nov 29, 2016, at 7:22 PM, Daniel Fuchs <daniel.fu...@oracle.com> wrote: Hi Max, On 29/11/16 06:46, Wang Weijun wrote: http://cr.openjdk.java.net/~weijun/8170408/webrev.00/ A lambda inside JDK is dumped, we should not count it in this test. Looks good to me. An alterna

Re: RFR 8170408: LogGeneratedClassesTest.java fails with recent changes

2016-11-29 Thread Daniel Fuchs
Hi Max, On 29/11/16 06:46, Wang Weijun wrote: http://cr.openjdk.java.net/~weijun/8170408/webrev.00/ A lambda inside JDK is dumped, we should not count it in this test. Looks good to me. An alternative might be to only look at the files under dump/com/example (I remember modifying

Re: RFR JDK-8170192 [JAXP] [TESTBUG] test/javax/xml/jaxp/libs/jaxp/library/JAXPPolicyManager.java should grant permissions to jtreg, javatest, and testng jars

2016-11-24 Thread Daniel Fuchs
Hi Frank, Looks good to me. Thanks for the advice Jon! best regards, -- daniel On 24/11/16 02:29, Frank Yuan wrote: Hi Jon Thank you for your advice! Please check the update http://cr.openjdk.java.net/~fyuan/8170192/webrev.01/ , which contains jcommander.jar and removes the extra blank

Re: RFR JDK-8170192 [JAXP] [TESTBUG] test/javax/xml/jaxp/libs/jaxp/library/JAXPPolicyManager.java should grant permissions to jtreg, javatest, and testng jars

2016-11-23 Thread Daniel Fuchs
Hi Frank, Thanks for taking this on. Looks good to me. -- daniel On 23/11/16 04:41, Frank Yuan wrote: Hi All Would you like to review http://cr.openjdk.java.net/~fyuan/8170192/webrev.00/? Bug: https://bugs.openjdk.java.net/browse/JDK-8170192 This patch is fully same as Daniel provided

Re: Did the tests fail due to TestNG dataprovider parameter type check?

2016-11-23 Thread Daniel Fuchs
-libs-dev@openjdk.java.net; jtreg-...@openjdk.java.net; 'Volker Simonis'; 'Daniel Fuchs' Subject: RE: Issues running JAXP jtreg tests ("java.lang.RuntimePermission" "accessDeclaredMembers") Thanks, Frank. we run scheduled jtreg tests for jdk every night so we should have encou

Re: RFR[9] JDK-8158916: ProblemList.txt update for com/sun/jndi/ldap/DeadSSLLdapTimeoutTest.java

2016-11-23 Thread Daniel Fuchs
,macosx-all +com/sun/jndi/ldap/DeadSSLLdapTimeoutTest.java 8169942 linux-i586,macosx-all Best regards, John Jiang On 2016/11/8 18:53, Daniel Fuchs wrote: Hi John, On 08/11/16 01:30, John Jiang wrote: Hi Daniel

Re: Issues running JAXP jtreg tests ("java.lang.RuntimePermission" "accessDeclaredMembers")

2016-11-22 Thread Daniel Fuchs
Hi Christoph, I have logged https://bugs.openjdk.java.net/browse/JDK-8170192 best regards, -- daniel On 22/11/16 14:47, Daniel Fuchs wrote: On 22/11/16 14:43, Langer, Christoph wrote: But, as for fixing with the new testng, will you look into this? Shall I open a bug?

Re: Issues running JAXP jtreg tests ("java.lang.RuntimePermission" "accessDeclaredMembers")

2016-11-22 Thread Daniel Fuchs
On 22/11/16 14:51, Langer, Christoph wrote: In that case, if we can't change testng, maybe the jaxp SecurityManager can allow testng to access the declared members without granting this to the testee code? That's what I was prototyping. The patch below seem to fix the issue - but it's a bit

Re: Issues running JAXP jtreg tests ("java.lang.RuntimePermission" "accessDeclaredMembers")

2016-11-22 Thread Daniel Fuchs
On 22/11/16 14:43, Langer, Christoph wrote: But, as for fixing with the new testng, will you look into this? Shall I open a bug? I am unsure on how to fix this actually. The new testng must attempt to load the data provider after having invoked the listener, while the old testng probably did

Re: Issues running JAXP jtreg tests ("java.lang.RuntimePermission" "accessDeclaredMembers")

2016-11-22 Thread Daniel Fuchs
Hi Volker, Our emails crossed each others :-) On 22/11/16 14:31, Volker Simonis wrote: @Daniel: can I please kindly ask you to retry your tests with https://adopt-openjdk.ci.cloudbees.com/job/jtreg/lastSuccessfulBuild/artifact/jtreg-4.2-b03.tar.gz and make sure you are really using that

Re: Issues running JAXP jtreg tests ("java.lang.RuntimePermission" "accessDeclaredMembers")

2016-11-22 Thread Daniel Fuchs
-4.2-b03.tar.gz which has testng 6.9.11. When I dowloaded http://mvnrepository.com/artifact/org.testng/testng/6.9.5 and installed it in the jtreg install dir, the issue disappeared. best regards, -- daniel On 22/11/16 13:31, Daniel Fuchs wrote: Can it be that we are using a testng framework

Re: Issues running JAXP jtreg tests ("java.lang.RuntimePermission" "accessDeclaredMembers")

2016-11-22 Thread Daniel Fuchs
On 22/11/16 13:31, Daniel Fuchs wrote: On 22/11/16 13:01, Langer, Christoph wrote: Hi, we are running jtreg with something like -jdk:/images/jdk. So would that be the exploded version or not? Yes - that's the image. Hmm... The failures I see with the exploded build are different than what

Re: Issues running JAXP jtreg tests ("java.lang.RuntimePermission" "accessDeclaredMembers")

2016-11-22 Thread Daniel Fuchs
em running the tests with that version of testng (except if I use the exploded build). Maybe the listener that the test uses to set up the security manager is invoked in a different way (earlier?) with the 6.9.10 version? best regards, -- daniel Best, Christoph -Original Message

Re: Issues running JAXP jtreg tests ("java.lang.RuntimePermission" "accessDeclaredMembers")

2016-11-22 Thread Daniel Fuchs
Hi guys, Are you running the tests with the exploded jdk or with the image? I'm seeing failures with the exploded jdk. That could explain the difference with permission checks. best regards, -- daniel On 22/11/16 12:32, Daniel Fuchs wrote: Hi Volker, On 22/11/16 12:25, Chris Hegarty

Re: Issues running JAXP jtreg tests ("java.lang.RuntimePermission" "accessDeclaredMembers")

2016-11-22 Thread Daniel Fuchs
mission, but you objected (http://mail.openjdk.java.net/pipermail/core-libs-dev/2016-July/042520.html) and it was removed in the final version. Any more hints? Thanks, Volker On Tue, Nov 22, 2016 at 12:24 PM, Daniel Fuchs <daniel.fu...@oracle.com> wrote: Hi Christoph, Is there anything

Re: Issues running JAXP jtreg tests ("java.lang.RuntimePermission" "accessDeclaredMembers")

2016-11-22 Thread Daniel Fuchs
Hi Christoph, Is there anything funny with the place jtreg is installed? like: - path contains whitespaces - path is accessible through links or mount points... It seems clear that the issue here is that testng classes are missing some permissions, so I was wondering whether that could be

Re: JDK 9 RFR of JDK-8170158: Remove ClassLoader/platformClassLoader/DefinePlatformClass.java from ProblemList

2016-11-22 Thread Daniel Fuchs
Looks good Amy! best regards, -- daniel On 22/11/16 03:26, Amy Lu wrote: Please review the patch to bring back java/lang/ClassLoader/platformClassLoader/DefinePlatformClass.java It's clear now that the reported failure was caused by machine wrong time setting. bug:

Re: RFR(s): 8169721: [TESTBUG] com/sun/jndi tests have undeclared dependency on java.naming module

2016-11-18 Thread Daniel Fuchs
On 18/11/16 14:09, Sergei Kovalev wrote: Hi Daniel, Thank you for feedback. You absolutely right about modules. To have an ability to distinguish environment issue from test pass I've added a warning message how you recommended. At least it will have a visible effect.

Re: RFR[9] JDK-8158916: ProblemList.txt update for com/sun/jndi/ldap/DeadSSLLdapTimeoutTest.java

2016-11-18 Thread Daniel Fuchs
linux-i586,macosx-all +com/sun/jndi/ldap/DeadSSLLdapTimeoutTest.java 8169942 linux-i586,macosx-all Best regards, John Jiang On 2016/11/8 18:53, Daniel Fuchs wrote: Hi John, On 08/11/16 01:30, John

Re: [JAXP] RFR: 8169778: Add new public methods to get new instances of the JAXP factories builtin system-default implementations

2016-11-17 Thread Daniel Fuchs
e a properly formed link. XMLOutputFactoryNewInstanceTest - line 2, has a 1999 date but is a new file. Many of the files have long lines (some new) that make side-by-side compares not fit on the screen. Regards, Roger On 11/16/2016 10:24 AM, Daniel Fuchs wrote: Hi, Please find below

Re: RFR (JAXP) 8158619: Very large CDATA section in XML document causes OOME

2016-11-17 Thread Daniel Fuchs
Looks good to me Joe. best regards, -- daniel On 17/11/16 19:58, Joe Wang wrote: Thanks Lance, Daniel, and Christoph. I adopted the changes as suggested. http://cr.openjdk.java.net/~joehw/jdk9/8158619/webrev/ The changeset has been in my workspace for over a month now. So it's good to

Re: RFR(s): 8169721: [TESTBUG] com/sun/jndi tests have undeclared dependency on java.naming module

2016-11-17 Thread Daniel Fuchs
Hi Sergey, I added a System.out.println in UnbindIdempotent (see diff below) to double check which type of context object was expected: Got context: class com.sun.jndi.rmi.registry.RegistryContext This lets me think that this test does not depend on java.naming but rather on jdk.naming.rmi -

Re: RFR (JAXP) 8158619: Very large CDATA section in XML document causes OOME

2016-11-17 Thread Daniel Fuchs
Hi Joe, Good to see some raw type uses corrected :-) nit: there are some very long lines that could be split to help side-by-side reviewing, like e.g. in XSDHandler.java, Parser.java, etc... I know that most of these files already have long lines (and some of them would probably

Re: [JAXP] RFR: 8169778: Add new public methods to get new instances of the JAXP factories builtin system-default implementations

2016-11-16 Thread Daniel Fuchs
.openjdk.java.net/~dfuchs/webrev_8169778/webrev.01/index.html -- daniel Regards, Roger On 11/16/2016 10:24 AM, Daniel Fuchs wrote: Hi, Please find below a patch for: 8169778: Add new public methods to get new instances of the JAXP factories builtin system-default implementa

[JAXP] RFR: 8169778: Add new public methods to get new instances of the JAXP factories builtin system-default implementations

2016-11-16 Thread Daniel Fuchs
Hi, Please find below a patch for: 8169778: Add new public methods to get new instances of the JAXP factories builtin system-default implementations bug: https://bugs.openjdk.java.net/browse/JDK-8169778 webrev: http://cr.openjdk.java.net/~dfuchs/webrev_8169778/webrev.00/ best regards,

[JAXP] RFR 8169723: remove jaxp/src/java.xml/share/classes/org/w3c/dom/xpath/COPYRIGHT.html

2016-11-15 Thread Daniel Fuchs
Hi, Please find below a trivial patch for JBS: https://bugs.openjdk.java.net/browse/JDK-8169723 webrev: http://cr.openjdk.java.net/~dfuchs/webrev_8169723/webrev.00/ The org.w3c.dom.xpath package is now exported by the jdk.xml.dom package. Somehow this copyright file was copied instead of being

Re: JDK-Review Request: JDK-8169606 jdeps --list-reduced-deps should not show java.base as all modules require it

2016-11-12 Thread Daniel Fuchs
Hi Mandy, Looks good to me. cheers, -- daniel On 12/11/16 06:46, Mandy Chung wrote: Daniel, This is an improved version of what you reviewed earlier. jdeps --list-reduced-deps will show java.base only when it’s the only module it depends on.

Re: Review request: JDK-8168386: Fix jdeps verbose options

2016-11-11 Thread Daniel Fuchs
Hi Mandy, This looks good to me. However, if I'm not mistaken, jdeps will now no longer list java.base as a dependency, unless the dependency is on a jdk internal API. This might be worthy of some mention in release notes - if any tools are relying on jdeps output. best regards, -- daniel On

RFR: 8169495: Add a method to set an Authenticator on a HttpURLConnection.

2016-11-10 Thread Daniel Fuchs
Hi, Please find below a patch for: https://bugs.openjdk.java.net/browse/JDK-8169495 8169495: Add a method to set an Authenticator on a HttpURLConnection. webrev: http://cr.openjdk.java.net/~dfuchs/webrev_8169495/webrev.00 The public API changes are in java.net.HttpURLConnection and

Re: RFR[9] JDK-8158916: ProblemList.txt update for com/sun/jndi/ldap/DeadSSLLdapTimeoutTest.java

2016-11-08 Thread Daniel Fuchs
Hi John, On 08/11/16 01:30, John Jiang wrote: Hi Daniel, Thanks for your comments. On 2016/11/7 19:21, Daniel Fuchs wrote: Hi John, Maybe you should reach out to Rob who fixed JDK-8141370 to make sure this is the right thing to do. just cc Rob Mckenna It seems the exclusion

Re: RFR[9] JDK-8158916: ProblemList.txt update for com/sun/jndi/ldap/DeadSSLLdapTimeoutTest.java

2016-11-07 Thread Daniel Fuchs
Hi John, Maybe you should reach out to Rob who fixed JDK-8141370 to make sure this is the right thing to do. It seems the exclusion on problematic platforms [1] was deliberate: Basically I've separated the failing subtest out into its own file and excluded it on the (intermittently) failing

Re: JDK 9 RFR of JDK-8037278: sun/rmi/runtime/Log/6409194/NoConsoleOutput.java fails Intermittently: unexpected subprocess output

2016-11-04 Thread Daniel Fuchs
, removed the test dependency on TestLibrary. (RMI logging still be triggered thus no change to testing purpose.) Please review again: http://cr.openjdk.java.net/~amlu/8037278/webrev.02/ Thanks, Amy On 11/3/16 9:42 PM, Daniel Fuchs wrote: On 03/11/16 13:35, Daniel Fuchs wrote: Hi Amy, Looks

Re: JDK 9 RFR of JDK-8037278: sun/rmi/runtime/Log/6409194/NoConsoleOutput.java fails Intermittently: unexpected subprocess output

2016-11-03 Thread Daniel Fuchs
On 03/11/16 13:35, Daniel Fuchs wrote: Hi Amy, Looks good to me. Maybe a followup should be logged to try and no longer call TestLibrary.getUnusedRandomPort(), as this the method that is causing the rogue output (and I see that it might print things to System.err as well). best regards

Re: JDK 9 RFR of JDK-8037278: sun/rmi/runtime/Log/6409194/NoConsoleOutput.java fails Intermittently: unexpected subprocess output

2016-11-03 Thread Daniel Fuchs
Hi Amy, Looks good to me. -- daniel On 03/11/16 13:22, Amy Lu wrote: Thank you Daniel for your review. Yes, agree and reverted those three lines. webrev updated: http://cr.openjdk.java.net/~amlu/8037278/webrev.01 Thanks, Amy On 11/3/16 8:37 PM, Daniel Fuchs wrote: Hi Amy, I think

Re: JDK 9 RFR of JDK-8037278: sun/rmi/runtime/Log/6409194/NoConsoleOutput.java fails Intermittently: unexpected subprocess output

2016-11-03 Thread Daniel Fuchs
Hi Amy, I think it might still be useful to see what the subprocess prints on System.out in case of failure. I would leave lines 83, 86 and 87 unchanged. best regards, -- daniel On 03/11/16 11:58, Amy Lu wrote: Please review the patch for test sun/rmi/runtime/Log/6409194/NoConsoleOutput.java

Re: RFR(s): 8169055: [TESTBUG] : java/io/Serializable/serialFilter/ tests have undeclared dependency on jdk.jdeps module

2016-11-02 Thread Daniel Fuchs
-8169055/ +1 -- daniel Roger On 11/2/2016 9:31 AM, Sergei Kovalev wrote: Thank you all, java.compiler is enough. http://cr.openjdk.java.net/~skovalev/8169055/webrev.02/ 02.11.16 16:25, Daniel Fuchs wrote: Hi Sergey, Why jdk.compiler? Why not simply java.compiler? best regards

Re: RFR(s): 8169055: [TESTBUG] : java/io/Serializable/serialFilter/ tests have undeclared dependency on jdk.jdeps module

2016-11-02 Thread Daniel Fuchs
Hi Sergey, Why jdk.compiler? Why not simply java.compiler? best regards, -- daniel On 02/11/16 12:51, Sergei Kovalev wrote: Fixed http://cr.openjdk.java.net/~skovalev/8169055/webrev.01/ 02.11.16 15:34, Chris Hegarty wrote: On 2 Nov 2016, at 12:31, Sergei Kovalev

Re: RFR(s): 8169055: [TESTBUG] : java/io/Serializable/serialFilter/ tests have undeclared dependency on jdk.jdeps module

2016-11-02 Thread Daniel Fuchs
ut jdk.compiler. What should I change in my environment to highlight dependency from the module? jdk.jdeps requires java.compiler, and IMO this is why this working. But as far as I can tell nothing depends on jdk.jdeps in these tests. cheers, -- daniel 02.11.16 14:45, Daniel Fuchs wrote:

Re: RFR(s): 8169055: [TESTBUG] : java/io/Serializable/serialFilter/ tests have undeclared dependency on jdk.jdeps module

2016-11-02 Thread Daniel Fuchs
Hi Sergey, I do not see any dependency on jdk.jdeps in the test sources. What I do see is a dependency on java.compiler though. So why @modules jdk.jdeps and not @modules java.compiler? best regards, -- daniel On 02/11/16 11:22, Sergei Kovalev wrote: Hi team, Please review a very small

Re: RFR 9: JDK-8168862:Tighten permissions granted to the jdk.zipfs module

2016-11-01 Thread Daniel Fuchs
Hi Sherman, Disclaimer: I don't know anything about zipfs. However, I find it suspicious that it doesn't need a read permission for the "user.dir" system property. Isn't that a permission that any file system provider would need? My experience is that tests which deal with files and

Re: RFR: 8152515: (logging) LogManager.resetLogger should ignore LinkageError

2016-11-01 Thread Daniel Fuchs
closing a handler? Keep going... +} catch (Error e) { +// ignore Errors while shutting down +if (globalHandlersState != STATE_SHUTDOWN) { +throw e; +} } best regards, -- daniel Cheers, David On 28/10/2016 9:51 PM, Da

Re: 8152515: (logging) LogManager.resetLogger should ignore LinkageError

2016-10-28 Thread Daniel Fuchs
s for fixing this! Jason ____ From: Daniel Fuchs <daniel.fu...@oracle.com <mailto:daniel.fu...@oracle.com>> Sent: Friday, October 28, 2016 6:51 AM To: core-libs-dev Cc: Jason Mehrens Subject: RFR: 8152515: (logging) LogMa

Re: RFR: 8152515: (logging) LogManager.resetLogger should ignore LinkageError

2016-10-28 Thread Daniel Fuchs
this test if the fix is not present? Here is the webrev with the test: http://cr.openjdk.java.net/~dfuchs/webrev_8152515/webrev.01 -- daniel On 10/28/2016 7:51 AM, Daniel Fuchs wrote: Hi, Please find below a trivial patch for: 8152515: (logging) LogManager.resetLogger should ignore

RFR: 8152515: (logging) LogManager.resetLogger should ignore LinkageError

2016-10-28 Thread Daniel Fuchs
Hi, Please find below a trivial patch for: 8152515: (logging) LogManager.resetLogger should ignore LinkageError https://bugs.openjdk.java.net/browse/JDK-8152515 Patch: http://cr.openjdk.java.net/~dfuchs/webrev_8152515/webrev.00/ The issue might occur at shutdown, when a handler that makes

Re: Review Request: JDK-8167057 jdeps to list the modules and internal APIs to help find @modules for tests

2016-10-28 Thread Daniel Fuchs
Hi Mandy, Looks good to me in general, but I feel like the new option --list-reduced-deps should be better documented: jdeps.properties: 152 main.opt.list-deps=\ 153 \ --list-deps\n\ 154 \ --list-reduced-deps Lists the dependences and use of JDK internal\n\ 155 \

Re: RFR: 8163162: The separation between system loggers and application loggers should take the extension loader in consideration.

2016-10-27 Thread Daniel Fuchs
On 27/10/16 00:17, Mandy Chung wrote: On Oct 26, 2016, at 6:58 AM, Daniel Fuchs <daniel.fu...@oracle.com> wrote: With the deprivileging of some JDK modules, classes loaded by the Platform class loader should get the same kind of loggers than classes loaded by the Boot class loader

Re: RFR: 8163162: The separation between system loggers and application loggers should take the extension loader in consideration.

2016-10-26 Thread Daniel Fuchs
AM, Daniel Fuchs wrote: Hi, Please find below a small patch for 8163162: The separation between system loggers and application loggers should take the extension loader in consideration. https://bugs.openjdk.java.net/browse/JDK-8163162 With the deprivileging of some JDK modules, classes

RFR: 8163162: The separation between system loggers and application loggers should take the extension loader in consideration.

2016-10-26 Thread Daniel Fuchs
Hi, Please find below a small patch for 8163162: The separation between system loggers and application loggers should take the extension loader in consideration. https://bugs.openjdk.java.net/browse/JDK-8163162 With the deprivileging of some JDK modules, classes loaded by the Platform

Re: RFR (JAXP) 8152530: NullPointerException when xmlns=""

2016-10-11 Thread Daniel Fuchs
Looks good to me Joe! best regards, -- daniel On 11/10/16 21:29, Joe Wang wrote: Hi, Please review a fix for a NPE when the source contains empty namespace. JBS: https://bugs.openjdk.java.net/browse/JDK-8152530 webrev: http://cr.openjdk.java.net/~joehw/jdk9/8152530/webrev/ Thanks, Joe

Re: RFR: 8062389,8029459,8061950: Class.getMethod() is inconsistent with Class.getMethods() results + more

2016-10-11 Thread Daniel Fuchs
Hi Peter, I was looking at the test - I didn't see a case where the same method would be declared by two unrelated interfaces later implemented by the same class. Do we have test cases to verify that: public interface I1 { public Object test(String blah); } public interface I2 { public

Re: RFR: 8167002: JAXP schema validator: Use HashSet instead of ArrayList for tracking XML IDs

2016-10-03 Thread Daniel Fuchs
Hi Martin, This looks good to me. best regards, -- daniel On 01/10/16 01:41, Martin Buchholz wrote: https://bugs.openjdk.java.net/browse/JDK-8167002 http://cr.openjdk.java.net/~martin/webrevs/openjdk9/xml-id-validation-by-hash/

Re: RFR: 8166398: CatalogSupport tests need to be fixed -> Re: RFR: 8166220: Catalog API: JAXP XML Processor Support - add StAX test coverage

2016-09-22 Thread Daniel Fuchs
, removed a ObjectFactory call in the course. JBS: https://bugs.openjdk.java.net/browse/JDK-8166398 webrev: http://cr.openjdk.java.net/~joehw/jdk9/8166398/webrev/ Thanks, Joe On 9/20/16, 11:22 AM, Joe Wang wrote: On 9/20/16, 2:20 AM, Daniel Fuchs wrote: Hi Joe, test/javax/xml/jaxp/unittest

Re: RFR: 8166220: Catalog API: JAXP XML Processor Support - add StAX test coverage

2016-09-20 Thread Daniel Fuchs
Hi Joe, test/javax/xml/jaxp/unittest/catalog/CatalogSupportBase.java 603 /** 604 * Returns the text of the first element found by the reader. 605 * @param streamReader the XMLStreamReader 606 * @return the text of the first element 607 * @throws XMLStreamException

Re: RFR: JDK-8134373: explore potential uses of convenience factories within the JDK

2016-09-15 Thread Daniel Fuchs
Hi, I'm not sure I like the replacement of Collections.emptyList() by List.of(); I find emptyList() more expressive (+ it always returns the same instance). best regards, -- daniel On 15/09/16 11:46, Pavel Rappo wrote: Hi, I have had a look at the change. It looks good. Retrofitting

Re: RFR(JAXP): 8165784: Deprecate the internal Catalog API in JDK 9

2016-09-14 Thread Daniel Fuchs
/14/2016 5:11 AM, Daniel Fuchs wrote: Hi Joe, As much as I would like to support removing obsolete classes, I wonder if the forRemoval=true is a bit premature. I see that for instance, com.sun.org.apache.xml.internal.resolver.Catalog seems to be still used at many places in our code base

Re: JDK 9 RFR of JDK-8166054: Problem list JarURLConnectionUseCaches.java until JDK-8165988 is fixed

2016-09-14 Thread Daniel Fuchs
Hi Joe, Looks good to me! -- daniel On 14/09/16 18:07, joe darcy wrote: Hello, Until JDK-8165988 is fixed, the test sun/net/www/protocol/jar/JarURLConnectionUseCaches.java should be problem listed on windows. Please review the patch below. Thanks, -Joe --- a/test/ProblemList.txt

Re: RFR 9: 8165261: RMI API to export an object with a serialization filter

2016-09-14 Thread Daniel Fuchs
Hi Roger, On 13/09/16 20:10, Roger Riggs wrote: Hi Daniel, Thanks for the review and comments... Webrev updated in place. Looks good to me! best regards, -- daniel

Re: RFR(JAXP): 8165784: Deprecate the internal Catalog API in JDK 9

2016-09-14 Thread Daniel Fuchs
Hi Joe, As much as I would like to support removing obsolete classes, I wonder if the forRemoval=true is a bit premature. I see that for instance, com.sun.org.apache.xml.internal.resolver.Catalog seems to be still used at many places in our code base. It would be more convincing if we could

Re: Review Request: JDK-8157464: StackWalker.getCallerClass() is not

2016-09-14 Thread Daniel Fuchs
Hi Mandy, webrev.03 looks good to me! best regards, -- daniel On 14/09/16 01:04, Mandy Chung wrote: Yes that’s one option. JVM_STACKWALK_FILL_CLASS_REFS_ONLY is not necessarily used to get caller class. For example, AccessControlContext is interested in protection domains. We could build

Re: RFR(JAXP): 8165784: Deprecate the internal Catalog API in JDK 9

2016-09-14 Thread Daniel Fuchs
Hi Joe, I will review it in more details but here is an early feedback: 66 * href="http://download.java.net/java/jdk9/docs/api/javax/xml/catalog/package-summary.html;> 67 * new public API. The following should work (it did in the past, might be worth checking that it still does): *

Re: Review Request: JDK-8157464: StackWalker.getCallerClass() is not

2016-09-13 Thread Daniel Fuchs
Hi Mandy, This looks good to me. But I wonder about these 5 lines - isn't this introducing a change of behavior if the caller is an anonymous class? 149 InstanceKlass* ik = method->method_holder(); 150 if (ik->is_anonymous()) { 151 // use the host class as the caller

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

2016-09-13 Thread Daniel Fuchs
Thanks Mandy! On 13/09/16 15:56, Mandy Chung wrote: The new webrev.05 looks good. On Sep 13, 2016, at 5:45 AM, Daniel Fuchs <daniel.fu...@oracle.com> wrote: Hi Mandy, Here is a new webrev with your feedback integrated. Finally I managed to get rid of the InternalError in Method

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

2016-09-13 Thread Daniel Fuchs
/16 21:10, Mandy Chung wrote: On Aug 29, 2016, at 6:10 AM, Daniel Fuchs <daniel.fu...@oracle.com> wrote: Here is a new webrev that uses ClassLoaderValue as Peter suggested. http://cr.openjdk.java.net/~dfuchs/webrev_6543126/webrev.04/ Looks good in general. 553 // KnowLevelCL

Re: RFR 9: 8165261: RMI API to export an object with a serialization filter

2016-09-13 Thread Daniel Fuchs
Hi Roger, On 12/09/16 21:42, Roger Riggs wrote: Please review an update to enable serialization filtering for exported RMI objects. Webrev: http://cr.openjdk.java.net/~rriggs/webrev-rmi-filter-8165261/ Issue: https://bugs.openjdk.java.net/browse/JDK-8165261 Thanks, Roger In

Re: RFR 9: 8155760 Implement Serialization Filtering

2016-09-13 Thread Daniel Fuchs
/SerializablePermission.html On 9/12/2016 1:15 PM, Daniel Fuchs wrote: Hi Roger, ObjectInputStream.java: some cosmetic comments: 317 * {@link ObjectInputFilter.Config#getSerialFilter() the process-wide filter}. 352 * {@link ObjectInputFilter.Config#getSerialFilter() the process-wide filter

Re: RFR 9: 8155760 Implement Serialization Filtering

2016-09-12 Thread Daniel Fuchs
Hi Roger, ObjectInputStream.java: some cosmetic comments: 317 * {@link ObjectInputFilter.Config#getSerialFilter() the process-wide filter}. 352 * {@link ObjectInputFilter.Config#getSerialFilter() the process-wide filter}. => should be @linkplain 1185 * The filter, when not

Re: RFR (JAXP) 8161454: Fails to Load external Java method from inside of a XSL stylesheet if SecurityManager is present

2016-09-01 Thread Daniel Fuchs
On 31/08/16 21:47, Joe Wang wrote: Thanks Aleksej! Hi Joe, Your last revision looks good. best, -- daniel -Joe On 8/31/16, 11:26 AM, Aleks Efimov wrote: Hi Joe, The changes looks nice (I'm not a reviewer) Best Regards, Aleksej On 31/08/16 19:47, Joe Wang wrote: Hi, Please review

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

2016-08-29 Thread Daniel Fuchs
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 added an additional selector parame

Re: RFR: 8163232: CatalogResolver to extend XMLResolver and LSResourceResolver

2016-08-26 Thread Daniel Fuchs
Hi Joe, Looks good to me! -- daniel On 26/08/16 18:06, Joe Wang wrote: Hi, Please review a consolidation of resolver support to CatalogResolver. The purpose is to simplify the interface and usage scenarios so that the one CatalogResolver is usable for all JDK XML processors. As with the

Re: Can't get Platform Logging to work

2016-08-19 Thread Daniel Fuchs
Hi Nicolai, On 19/08/16 16:21, Nicolai Parlog wrote: Hi Daniel, thanks for the quick reply. And you are of course right. Luckily I don't care about the specific G1 message. After setting everything up, I was looking for _anything_ that logs a message and that was the first thing I came up

Re: Can't get Platform Logging to work

2016-08-19 Thread Daniel Fuchs
Hi Nicolai, On 19/08/16 16:05, Nicolai Parlog wrote: On a tangent, I also wondered what would be the preferred way to log from inside my code. Should I use System::getLogger directly? Logger logger = System.getLogger("Application"); logger.log(Level.INFO, "Hello, World!"); so

Re: Can't get Platform Logging to work

2016-08-19 Thread Daniel Fuchs
Hi Nicolai, Log messages emitted by the JVM are not emitted through the System.Logger API. JEP 264 is a pure Java API which aims at replacing the sun.util.logging.PlatformLogger implementation. I believe what you are looking for is JEP 158 Unified JVM Logging [1] best regards, -- daniel [1]

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

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
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
Hi Mandy, On 17/08/16 05:05, Mandy Chung wrote: On Aug 16, 2016, at 5:42 AM, Daniel Fuchs <daniel.fu...@oracle.com> wrote: > > Hi Mandy, > > I added an additional selector parameter to the find methods. > This made it possible to return Optional instead of > KnownLe

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

2016-08-16 Thread Daniel Fuchs
Hi Peter, On 16/08/16 13:52, Peter Levart wrote: Ah, I see. Right. Of course, my bad. In that case, there is a possible race that could lead to exception here: 476 // add new Level. 477 Level levelObject = new Level(name, x); 478 return

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

2016-08-16 Thread Daniel Fuchs
On 16/08/16 13:59, Peter Levart wrote: Hi Daniel, Another place that is not clear to me is the following (in old code): 585 // Returns a KnownLevel with the given localized name matching 586 // by calling the Level.getLocalizedLevelName() method (i.e. found 587 //

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-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-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: 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 (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: [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: [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 (JAXP) JDK-8067170: Enable security manager on JAXP unit tests

2016-08-03 Thread Daniel Fuchs
://cr.openjdk.java.net/~fyuan/8067170/webrev.03/ Thanks Frank -Original Message- From: Joe Wang [mailto:huizhe.w...@oracle.com] Subject: Re: RFR (JAXP) JDK-8067170: Enable security manager on JAXP unit tests On 8/2/16, 5:30 AM, Daniel Fuchs wrote: Hi Frank, I am intrigued by these change which do

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: 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 (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: 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: 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 (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): 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 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 [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] 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 [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: 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: 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

<    4   5   6   7   8   9   10   11   12   13   >