Hi Jason,
On 08/06/16 15:31, Jason Mehrens wrote:
Hi Daniel,
Thanks for taking this on. Looks good.
This might be a new issue altogether but, if record.getMessage returns null a
NPE can be generated during 'catalog.getString' (per RB.handleGetObject
contract) or 'java.text.MessageFormat.format' (undocumented)
It is handled by the catch clause so it is harmless in terms of correctness.
The most common form of this abuse I've seen in the wild is
'logger.log(SEVERE, null, (Throwable) ioe);
So if it not worth explicitly checking for null maybe we should add a comment
explaining the intent that NPE could be generated and is trapped.
For sure having tests for null message with and without a resource bundle would
be a good idea.
Thanks again for pointing that out.
I thought we already had tests for that but it appear we don't.
The package.html says:
<<
In general, unless otherwise noted in the javadoc, methods and
constructors will throw NullPointerException if passed a null argument.
The one broad exception to this rule is that the logging convenience
methods in the Logger class (the config, entering, exiting, fine, finer,
finest,
log, logp, logrb, severe, throwing, and warning methods)
will accept null values
for all arguments except for the initial Level argument (if any).
>>
I am afraid this is both not true and not tested (sigh).
The new methods that accept a Supplier<String> may throw
a NPE for instance if the supplier is null and the level is
enabled. So it looks that we need to make yet another decision
about how to bring the spec and implementation in sync here,
and we definitely need more tests.
I will log a new issue for this - as as you say it appears
to have little to do with what this patch is about.
best regards,
-- daniel
Thanks,
Jason
From: core-libs-dev <core-libs-dev-boun...@openjdk.java.net> on behalf of Daniel
Fuchs <daniel.fu...@oracle.com>
Sent: Wednesday, June 8, 2016 8:46 AM
To: core-libs-dev
Subject: RFR: 8153666: Optimize Formatter.formatMessage
Hi,
Please find below a patch for a small optimization
in Formatter.formatMessage.
This patch also removed the synchronized keyword which
was there for historical reasons - but which has
become needless.
More background available at:
https://bugs.openjdk.java.net/browse/JDK-8153666
(thanks Martin!)
and
http://stackoverflow.com/questions/36433146/why-is-the-formatmessage-method-in-java-util-logging-formatter-synchronized
(thanks Jason!)
bug:
8153666: Optimize Formatter.formatMessage
https://bugs.openjdk.java.net/browse/JDK-8153666
patch:
http://cr.openjdk.java.net/~dfuchs/webrev_8153666/webrev.00
best regards, and thanks to all who provided suggestions and
archeological background!
-- daniel
