Re: [9] RFR: 8177136: Caller sensitive method System.getLogger should specify what happens if there is no caller on the stack.

Hi Mandy, On 22/03/2017 16:26, Mandy Chung wrote: I agree this is better. Also “caller” has been used in existing specs and no need to have the link to StackWalker#getCallerClass which seems not directly relevant here. Mandy done:

Re: [9] RFR: 8177136: Caller sensitive method System.getLogger should specify what happens if there is no caller on the stack.

Hi David, This is a good suggestion. Here is an updated webrev, that also incorporates Brent's feedback. http://cr.openjdk.java.net/~dfuchs/webrev_8177136/webrev.03 Best regards, -- daniel On 22/03/2017 00:09, David Holmes wrote: Hi Daniel, On 22/03/2017 4:41 AM, Daniel Fuchs wrote: Hi

Re: [9] RFR: 8177136: Caller sensitive method System.getLogger should specify what happens if there is no caller on the stack.

the system {@link * LoggerFinder#getLoggerFinder() LoggerFinder} to * obtain a logger. Note that doing the latter * may eagerly initialize the underlying logging * system. Thanks, -Brent On 3/21/17 5:44 AM, Daniel Fuchs wrote: Hi, Please find below

Re: [9] RFR: 8177136: Caller sensitive method System.getLogger should specify what happens if there is no caller on the stack.

#getLoggerFinder() LoggerFinder} * to obtain a logger instead. Note however that doing the latter * may force the eager initialization of the underlying logging * system. best regards, -- daniel Thanks, -Brent On 3/21/17 5:44 AM, Daniel Fuchs wrote: Hi, Please

[9] RFR: 8177136: Caller sensitive method System.getLogger should specify what happens if there is no caller on the stack.

Hi, Please find below an updated patch for: 8177136: Caller sensitive method System.getLogger should specify what happens if there is no caller on the stack. https://bugs.openjdk.java.net/browse/JDK-8177136 webrev: http://cr.openjdk.java.net/~dfuchs/webrev_8177136/webrev.01

Re: RFR: 8177136: Caller sensitive methods Logger.getLogger, Logger.getAnonymousLogger, and System.getLogger should throw IllegalCallerException if there is no caller on the stack.

System::getLogger should specify what happens if there is no caller on the stack. https://bugs.openjdk.java.net/browse/JDK-8177136 I will send a new RFR (with the new title) and an updated patch for this. best regards, -- daniel On 20/03/2017 12:08, Daniel Fuchs wrote: Hi, Please find

Re: RFR: 8177136: Caller sensitive methods Logger.getLogger, Logger.getAnonymousLogger, and System.getLogger should throw IllegalCallerException if there is no caller on the stack.

On 20/03/17 21:28, David Holmes wrote: If you are going to introduce an API with such a constraint then the constraint needs to be clearly documented and the alternatives also documented IMHO. System.getLogger relies on a notion of "current module". Yes: Instances returned by this method

Re: RFR: 8177136: Caller sensitive methods Logger.getLogger, Logger.getAnonymousLogger, and System.getLogger should throw IllegalCallerException if there is no caller on the stack.

.getLogger and explicitly supply a module IMO calling directly System.getLogger is not appropriate in this case. best, -- daniel Mandy On Mar 20, 2017, at 5:08 AM, Daniel Fuchs <daniel.fu...@oracle.com> wrote: Hi, Please find below a patch for: 8177136: Caller s

Re: RFR: 8177136: Caller sensitive methods Logger.getLogger, Logger.getAnonymousLogger, and System.getLogger should throw IllegalCallerException if there is no caller on the stack.

On 20/03/2017 12:37, David Holmes wrote: What about those API's says there has to be a Java frame higher up. Why can't an attached thread request a reference to the logger? Hi David, Did you look at the webrev? 1582 * @throws IllegalCallerException if there is no caller frame, i.e. 1583

Re: RFR: 8177136: Caller sensitive methods Logger.getLogger, Logger.getAnonymousLogger, and System.getLogger should throw IllegalCallerException if there is no caller on the stack.

(where no Java frame is on the stack) can always insert its own wrapper (e.g. a call these methods from an application class and call that application class from JNI instead). best regards, -- daniel David - On 20/03/2017 10:08 PM, Daniel Fuchs wrote: Hi, Please find below a patch for: 81771

Re: RFR [9]: 8177036: Class.checkMemberAccess throws NPE when calling Class methods via JNI

Hi Peter, On 20/03/2017 12:01, Peter Levart wrote: Perhaps the best way to rectify those problems in one place would be for Reflection.getCallerClass() to return a special internal class in its own package, such as: jdk.internal.solitary.NoCaller ...when there is no caller. This would work

RFR: 8177136: Caller sensitive methods Logger.getLogger, Logger.getAnonymousLogger, and System.getLogger should throw IllegalCallerException if there is no caller on the stack.

Hi, Please find below a patch for: 8177136: Caller sensitive methods Logger.getLogger, Logger.getAnonymousLogger, and System.getLogger should throw IllegalCallerException if there is no caller on the stack. https://bugs.openjdk.java.net/browse/JDK-8177136 Caller sensitive methods

Re: RFR of JDK-8176563: @since value errors in apis of java.base/java.logging module

Hi Hamlin, The new version looks good to me. Maybe wait for review of the other interested parties ;-) best regards, -- daniel On 15/03/2017 04:34, Hamlin Li wrote: BTW, I did *remove* the unnecessary @since for ObjectInputFilter.checkInput. :-) Thank you -Hamlin On 2017/3/15 11:34,

Re: RFR of JDK-8176563: @since value errors in apis of java.base/java.logging module

Hi Hamlin, I had the same remark than Roger & others about ObjectInputFilter::checkInput. The changes to LogManager are OK. How did I forget that @since? Thanks for catching that! Don't count me as Reviewer for the other java.util changes though. best regards -- daniel On 14/03/2017 06:40,

Re: Review request 8153912: StackFrame::getFileName and StackFrame::getLineNumber not needed

or Throwable::getStackTrace performance regression. On Mar 13, 2017, at 1:57 PM, Daniel Fuchs <daniel.fu...@oracle.com <mailto:daniel.fu...@oracle.com>> wrote: Hi Ralph, On 13/03/17 19:06, Ralph Goers wrote: OK - I will do that when I fix the other bug. But if you can suggest so

Re: Review request 8153912: StackFrame::getFileName and StackFrame::getLineNumber not needed

Hi Ralph, On 13/03/17 19:06, Ralph Goers wrote: OK - I will do that when I fix the other bug. But if you can suggest some other way to bail out of the loop after I have found the correct StackFrame that would help as well. I'd suggest using filter + findFirst, possibly with a statefull

Re: Review request 8153912: StackFrame::getFileName and StackFrame::getLineNumber not needed

Hi Ralph, I see one issue in your benchmark: StackWalker walker = StackWalker.getInstance(StackWalker.Option.RETAIN_CLASS_REFERENCE); will return a new StackWalker every time it's called. I'd suggest to create the StackWalker only once and put it in a private (or package) static. This will

Re: Review request 8153912: StackFrame::getFileName and StackFrame::getLineNumber not needed

Hi Ralph, On 13/03/2017 04:25, Ralph Goers wrote: Sorry for not getting back sooner but I finally found some time to follow up. I took a look at https://www.sitepoint.com/deep-dive-into-java-9s-stack-walking-api/ and

Re: RFR(s): 8176304: Number of javax/management test fails due to undeclared dependencies

Hi Sergei, I have several observations: - AFAIK you don't need to list modules that are required by a module already listed. so for instance, if you list jdk.management.agent then you don't need to list java.management or java.management.rmi (unless you need qualified exports from

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

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: rmid on Unix fails with Exception - maybe aftermath of JDK-8173607 ??

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: rmid on Unix fails with Exception - maybe aftermath of JDK-8173607 ??

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: RFR: 8174735 Update JAX-WS RI integration to latest version

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: [9] RFR: 8173390: Investigate SymbolTable in SAXParser

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

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

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] JDK-8174025 Regression in XML Transform caused by JDK-8087303

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) 8169450: StAX parse error if there is a newline in xml declaration

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

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-8170192 Regression in XML Transform caused by JDK-8087303

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: 8173898: StackWalker.walk throws InternalError if called from a constructor invoked through reflection.

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(s): 8174194: Several java/lang tests failing due to undeclared module dependencies.

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(s): 8174194: Several java/lang tests failing due to undeclared module dependencies.

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: JDK 9: 8174128: [testbug] Remove implementation dependency from java.time TCK tests

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.

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: 8173898: StackWalker.walk throws InternalError if called from a constructor invoked through reflection.

::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.

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.

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

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

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

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: Ping: [JAXP] RFR: 8173602: TESTBUG: javax/xml/jaxp/unittest/transform/TransformerTest.java needs refactoring

. 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

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

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: [RFR] 8160768: Add capability to custom resolve host/domain names within the default JDNI LDAP provider

. 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

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

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: 8173607: JMX RMI connector should be in its own module

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: [RFR] 8160768: Add capability to custom resolve host/domain names within the default JDNI LDAP provider

/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: 8173607: JMX RMI connector should be in its own module

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

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

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

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

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] 8160768: Add capability to custom resolve host/domain names within the default JDNI LDAP provider

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: RFR 8170116: Remove qualified exports from java.base to java.corba

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

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: (JAXP) RFR: 8173111: Excessive recursion in EventFilterSupport when filtering over large number of XML events can cause StackOverflow

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: 8173111: Excessive recursion in EventFilterSupport when filtering over large number of XML events can cause StackOverflow

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 (JAXP) 8169827: javax/xml/jaxp/isolatedjdk/catalog/PropertiesTest.sh copied JDK failed

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: RFR 8173159, Problem list java/rmi/activation/ActivationGroup/downloadActivationGroup/DownloadActivationGroup.java on Windows

+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: 8172971: java.management could use System.Logger

Hi Mandy, On 19/01/17 19:07, Mandy Chung wrote: This test should have @modules java.logging and java.management. OK - I believed @modules java.management should already be in TEST.ROOT I will double check. If @modules is specified in a test, it will override the one in TEST.properties (I

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

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

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

8172971: java.management could use System.Logger

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: RFR 8172350: Typo in Timestamp.toString()

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 ---

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

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

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

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: [9] RFR: 8171139: Simplify ResourceBundle.CacheKey and ClassLoader may not be needed

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

On 12/01/17 21:20, Naoto Sato wrote: Realized moduleRef & callerRef would never be null with this new version. Thus modified the patch as: http://cr.openjdk.java.net/~naoto/8171139/webrev.03/ Hi Naoto, 619 this(src.getName(), src.getLocale(), src.getModule(),

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

Hi Naoto, If I'm not mistaken, CacheKey is not subclassed anywhere, so it could be final. I'm wondering whether it would be a better idea to implement CacheKey.clone() with a private (copy?) constructor. It would make it possible to make most fields final: I believe it would be beneficial to

Re: RFR (JAXP) 8171243 : CatalogManager.catalogResolver throws FileSystemNotFoundException with jar

Hi Joe, BaseEntry.java: 213 * @return The URI created from the specified uri 214 * @throws IllegalArgumentException if the specified uri is null, 215 * or a URL can not be created based on the specified base and uri 216 */ 217 URL verifyURI(String arg, URL base,

Re: RFR of JDK-8029360: java/rmi/transport/dgcDeadLock/DGCDeadLock.java failing intermittently

Hi Hamlin, DGCDeadLock.java: 61 public static boolean finished = false; 62 static DGCDeadLock test = new DGCDeadLock(); 63 static int registryPort = -1; 1. 'finished' and 'registryPort' should be volatile since they are written and read by multiple threads. 2. 'test'

Re: Review Request: JDK-8171418: Remove jdeps internal --include-system-modules option

On 19/12/16 19:03, Mandy Chung wrote: On Dec 19, 2016, at 10:26 AM, Daniel Fuchs <daniel.fu...@oracle.com> wrote: On 19/12/16 18:06, Mandy Chung wrote: Updated webrev: http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8171418/webrev.01 Looks good to me. A related question is w

Re: Review Request: JDK-8171418: Remove jdeps internal --include-system-modules option

On Dec 19, 2016, at 9:54 AM, Daniel Fuchs <daniel.fu...@oracle.com> wrote: Hi Mandy, DepsAnalyzer.java: 232 Collection modules = configuration.getModules().values(); I don't see where modules is used in this method. Should this line be removed? Removed. Lance also caught that.

Re: Review Request: JDK-8171418: Remove jdeps internal --include-system-modules option

Hi Mandy, DepsAnalyzer.java: 232 Collection modules = configuration.getModules().values(); I don't see where modules is used in this method. Should this line be removed? 163 if (filter.requiresFilter().isEmpty()) { 164 return archives.stream() 165

Re: Review request - 8169465: Deadlock in com.sun.jndi.ldap.pool.Connections

.02/ -Rob On 14/12/16 04:58, Daniel Fuchs wrote: Hi Rob, The expire(long) method is already synchronized, so there's no need to call synchronized(this) inside, unless you forgot to to remove synchronized from the method declaration? I wonder if 'conns' could be created

Re: RFR of JDK-8171133: java/rmi/registry/reexport/Reexport.java, there is a missing case check in createReg(..)

Hi Hamlin, Looks good, but I would suggest to rename the parameter 'remoteOk' into something more natural, like 'shouldFail'. This should better help to understand the logic in createReg, which otherwise appears a bit obscure. No need to regenerate the webrev. best regards, -- daniel On

Re: RFR (JAXP) 8170556: Warnings cleanup related to JDK-8167340

On 14/12/16 00:42, Joe Wang wrote: Thanks Christoph! I updated the webrev for the classes you mentioned below, in a few cases, used NetBeans' source format feature -- not for all of the classes though (esp. the crazily large XMLDocumentFragmentScannerImpl.java, it gets better though, overtime).

Re: Review request - 8169465: Deadlock in com.sun.jndi.ldap.pool.Connections

Hi Rob, The expire(long) method is already synchronized, so there's no need to call synchronized(this) inside, unless you forgot to to remove synchronized from the method declaration? I wonder if 'conns' could be created as a CopyOnWriteArrayList. Then maybe no synchronization would be needed?

Re: Review Request JDK-8170772: ResourceBundle improper caching causes tools/javadoc tests intermittently

Hi Peter, This is a bold proposal, I would be frightened to touch at this code :-) Good observations about the simplifications induced by taking the caller's module as part of the cache key (in particular getting rid of RBClassLoader.INSTANCE). I have imported your patch (had to fight a bit

Re: RFR 8168979: @implNote for invalid FilePermission

Hi Max, Don't count me as reviewer - but I see a mismatched comment in the file: 209 /** 210 * Creates FilePermission objects with special internals. 211 * See {@link FilePermCompat#newPermPlusAltPath(Permission)} and 212 * {@link

Re: RFR 8169389 : Use a bitmap to control StackTraceElement::toString format and save footprint

Hi Brent, This looks really good now! best regards, -- daniel On 10/12/16 01:16, Brent Christian wrote: On 12/07/2016 04:05 PM, Mandy Chung wrote: I suggest to add two utility methods rather than the has method: boolean dropClassLoaderName() boolean dropModuleVersion() Done.

Re: Review Request JDK-8170772: ResourceBundle improper caching causes tools/javadoc tests intermittently

Hi Mandy, It will be good to have Naoto's opinion on this. But what you propose makes sense to me. best regards, -- daniel On 09/12/16 16:49, Mandy Chung wrote: Naoto, Can you review this ResourceBundle caching fix? The caller module may be different than the specified module to

Re: RFR 8170984: java.util.logging might force the initialization of ResourceBundle class too early.

On 09/12/16 15:05, Alan Bateman wrote: On 09/12/2016 14:09, Daniel Fuchs wrote: Hi, Please find below a fix for: issue: https://bugs.openjdk.java.net/browse/JDK-8170984 8170984: java.util.logging might force the initialization of ResourceBundle class too early. webrev: http

RFR 8170984: java.util.logging might force the initialization of ResourceBundle class too early.

Hi, Please find below a fix for: issue: https://bugs.openjdk.java.net/browse/JDK-8170984 8170984: java.util.logging might force the initialization of ResourceBundle class too early. webrev: http://cr.openjdk.java.net/~dfuchs/webrev_8170984/webrev.00/ The issue here is that

Re: RFR 9: 8170291 : Unpredictable results of j.i.ObjectInputFilter::createFilter

nly when reading objects from the stream and for + * not primitives. Thanks, Roger On 12/7/2016 6:46 AM, Daniel Fuchs wrote: Hi Roger, What about adding a bit of leeway in the reason for which IAE can be thrown. Here is the text from your webrev: 359 * @throws IllegalArgumentException If any of the

Re: Ping - Re: RFR 8043838, Test java/net/ServerSocket/AcceptCauseFileDescriptorLeak.java failed intermittently in nightly

On 07/12/16 14:50, Chris Hegarty wrote: It is intended, as that is the code path that will be executed when the test invokes itself. Oh - right - I see it now. So looks good to me too :-) -- daniel -Chris. best regards, -- daniel On 07/12/16 14:29, Felix Yang wrote: :-) -Felix On

Re: Ping - Re: RFR 8043838, Test java/net/ServerSocket/AcceptCauseFileDescriptorLeak.java failed intermittently in nightly

Hi Felix, Looks good in general, but is 74 return; intended, or is that a test bug? best regards, -- daniel On 07/12/16 14:29, Felix Yang wrote: :-) -Felix On 6 Dec 2016, at 9:28 AM, Felix Yang wrote: Add core-libs. Thanks, Felix On 2016/12/5

Re: RFR(s): 8170664: SystemLoggerInPlatformLoader.java failing in case of module limitation

Looks good to me Sergei! Thanks for making this change. best regards, -- daniel On 07/12/16 12:26, Sergei Kovalev wrote: Hi Mandy, Daniel, Thank you for reviewing this. I've made a try to improve naming of variables. Also replaced row Class declaration. Please take a look.

Re: RFR JDK-8169948 [JAXP] Update ServiceProviderTest for newDefaultInstance() methods in JAXP factories

Hi Frank, Looks good to me! best regards, -- daniel On 07/12/16 03:15, Frank Yuan wrote: Hi all Would you like to review http://cr.openjdk.java.net/~fyuan/8169948/webrev.00/? Bug: https://bugs.openjdk.java.net/browse/JDK-8169948 This test update is because of JDK-8169778 Add new

Re: RFR 9: 8170291 : Unpredictable results of j.i.ObjectInputFilter::createFilter

Hi Roger, What about adding a bit of leeway in the reason for which IAE can be thrown. Here is the text from your webrev: 359 * @throws IllegalArgumentException If any of the following is true: 360 * 361 *if a limit is missing the name or the name is not

Re: RFR(s): 8170664: SystemLoggerInPlatformLoader.java failing in case of module limitation

On 06/12/16 17:30, Mandy Chung wrote: On Dec 6, 2016, at 1:36 AM, Sergei Kovalev wrote: Hi Daniel, Please take a look at http://cr.openjdk.java.net/~skovalev/8170664/webrev.01/ 109 boolean simleConsoleOnly =

Re: RFR 8169653: Restore ObjectInputStream::resolveClass call stack default search order

On 06/12/16 14:17, Chris Hegarty wrote: On 6 Dec 2016, at 13:00, Daniel Fuchs <daniel.fu...@oracle.com> wrote: Hi Chris, Looks good. I wonder about the two different links for platform class loader now. ... You’re right. Since getPlatformClassLoader has a link to the built-in

Re: RFR 8169653: Restore ObjectInputStream::resolveClass call stack default search order

Hi Chris, Looks good. I wonder about the two different links for platform class loader now. Should the first one perhaps include "nor its ancestor" in the link text? best regards, -- daniel On 06/12/16 12:25, Chris Hegarty wrote: On 6 Dec 2016, at 11:53, Daniel Fuchs

Re: RFR 8169653: Restore ObjectInputStream::resolveClass call stack default search order

in ObjectInputStream::latestUserDefinedLoader() doesn't seem to match the implementation. best regards, -- daniel On 06/12/16 11:38, Chris Hegarty wrote: On 6 Dec 2016, at 11:35, Daniel Fuchs <daniel.fu...@oracle.com> wrote: Hi Chris, Is that a typo? I see it at two places: otherwise, + *

Re: RFR 8169653: Restore ObjectInputStream::resolveClass call stack default search order

Hi Chris, Is that a typo? I see it at two places: otherwise, + * loaded is the {@linkplain ClassLoader#getPlatformClassLoader() * platform class loader}. ("loaded" vs "loader") best regards, -- daniel On 06/12/16 11:27, Chris Hegarty wrote: Mandy and I exchanged some mails off

Re: RFR [9] 8166568 & 8169492 jmod extract and bug fix

Hi Chris, On 06/12/16 10:46, Chris Hegarty wrote: This change adds a basic option to the jmod tool to extract all its contents to the current working directory, 8166568 [1]. Additionally, there is a bug fix for a public mutable static, 8169492 [2]. This is not my area of expertise but these

Re: RFR of JDK-8170704: java/rmi/activation/Activatable/* tests fails intermittently with "output improperly annotated"

Looks reasonable to me Hamlin! best regards, -- daniel On 06/12/16 07:18, Hamlin Li wrote: Would you please review the below patch? bug: https://bugs.openjdk.java.net/browse/JDK-8170704 webrev: http://cr.openjdk.java.net/~mli/8170704/webrev.00/ * Root Cause: The issue can be reproduced by

Re: RFR(s): 8170664: SystemLoggerInPlatformLoader.java failing in case of module limitation

On 06/12/16 09:36, Sergei Kovalev wrote: Hi Daniel, Please take a look at http://cr.openjdk.java.net/~skovalev/8170664/webrev.01/ Hi Sergey, This looks good. Please add a comment to explain why we're doing this here: 111 if (simleConsoleOnly) { // happens if the

Re: RFR(s): 8170664: SystemLoggerInPlatformLoader.java failing in case of module limitation

On 05/12/16 14:50, Sergei Kovalev wrote: Hi Team, Could you please review a small change for regression test? Hi Sergei, Could you try with: if (Layer.boot().findModule("java.logging").isPresent()) { // expect JdkLazyLogger } else { // expect SimpleConsoleLogger } That shouldn't

Re: RFR (JAXP) 8167340: XMLStreamReader.getElementText return corrupt content when size of element is > 8192

long ones or any that impedes the reviews, that is, side-by-side view. +1 best regards, -- daniel Best, Joe On 12/1/16, 2:25 AM, Daniel Fuchs wrote: Hi Joe, I agree with Christoph's comments below. +1 best regards, -- daniel On 01/12/16 07:40, Langer, Christoph wrote: Hi Joe, to me

Re: RFR of JDK-8170644: java/rmi/registry/interfaceHash/InterfaceHash.java failed intermittently with "Port already in use"

Hi Hamlin, Looks good to me! best regards, -- daniel On 02/12/16 08:25, Hamlin Li wrote: Would you please review the below patch? bug: https://bugs.openjdk.java.net/browse/JDK-8170644 webrev: http://cr.openjdk.java.net/~mli/8170644/webrev.00/ Thank you -Hamlin

Re: RFR 8162521, java/net/Authenticator/B4933582.sh fails intermittently with BindException

On 01/12/16 14:40, Felix Yang wrote: Hi Daniel, please review the new webrev: http://cr.openjdk.java.net/~xiaofeya/8162521/webrev.01/ Looks good Felix! -- daniel Thanks, Felix On 2016/12/1 18:58, Daniel Fuchs wrote: Hi Felix, Good to see one more script converted to java. I wonder

Re: RFR 8162521, java/net/Authenticator/B4933582.sh fails intermittently with BindException

Hi Felix, Good to see one more script converted to java. I wonder if the fact that the test use to run two separate JVMs was significant. Hopefully it's not. The changes in TestHttpServer seem OK - this class is shared by many other tests - so any modifications here is to be taken with care. I

Re: RFR (JAXP) 8167340: XMLStreamReader.getElementText return corrupt content when size of element is > 8192

Hi Joe, I agree with Christoph's comments below. +1 best regards, -- daniel On 01/12/16 07:40, Langer, Christoph wrote: Hi Joe, to me this looks good. Did you already remove the cleanups from http://cr.openjdk.java.net/~joehw/jdk9/8167340/webrev/ ? I can't see a lot of them any more...

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