On 16/09/2014 16:31, Daniel Fuchs wrote:
Done.
http://cr.openjdk.java.net/~dfuchs/webrev_8043306/webrev.06
Thanks for the updates, I think this looks good now.
-Alan
On 15/09/2014 17:03, Daniel Fuchs wrote:
:
Here is a new webrev - with updated test case:
http://cr.openjdk.java.net/~dfuchs/webrev_8043306/webrev.05/
I think this quite good now.
Just on this wording:
If a listener terminates with an uncaught error or exception thenthe
first exception
Done.
http://cr.openjdk.java.net/~dfuchs/webrev_8043306/webrev.06
On 9/16/14 3:56 PM, Alan Bateman wrote:
On 15/09/2014 17:03, Daniel Fuchs wrote:
:
Here is a new webrev - with updated test case:
http://cr.openjdk.java.net/~dfuchs/webrev_8043306/webrev.05/
I think this quite good now.
On 12/09/2014 17:54, Daniel Fuchs wrote:
:
For this patch - I think that our options are limited to the alternative:
1. propagate the exception
2. catch and silentlty drop the exception
What is the 'better' behavior (and I agree neither of the two are ideal)
probably depends on what is the
On 09/10/2014 04:41 PM, Daniel Fuchs wrote:
On 9/10/14 4:28 PM, Jason Mehrens wrote:
Daniel
I think you should be able to remove the 'other instanceof
ConfigurationListener' branch in the ConfigurationListener.equals
method. Should work the same with or without that branch.
Oh yes. I put
On 09/11/2014 06:17 PM, Daniel Fuchs wrote:
On 9/10/14 1:25 PM, Alan Bateman wrote:
On 10/09/2014 10:49, Daniel Fuchs wrote:
Hi,
Please find below a proposed patch for
8043306 - Provide a replacement for the API that allowed to listen
for LogManager configuration changes
Hello,
Just a note, acc is going to leak the context class loader until the
listener is removed. That should be noted in the documentation.
Also if there is a runtime exception during run() of a listener it will
block any other listeners to be invoked and the exception is going to be
propagated
Hi Stanimir,
On 9/12/14 9:47 AM, Stanimir Simeonoff wrote:
Hello,
Just a note, acc is going to leak the context class loader until the
listener is removed. That should be noted in the documentation.
Good point. But I expect the concrete listener class will also be
usually loaded by the
Hi Peter,
On 9/12/14 8:26 AM, Peter Levart wrote:
Oh yes. I put it there just to avoid it being flagged by
NetBeans:
'equals() method not checking type of its parameters'
I guess it would be better to just return this == other;
(which unfortunately doesn't remove the warning)
What about
On 12/09/2014 08:14, Peter Levart wrote:
:
Just a question about security and delayed execution...
If at the time the configuration listener is added to the LogManager,
SecurityManager is not set, the listener will be invoked directly even
if at time the listener is invoked, SM has been set.
Good point. But I expect the concrete listener class will also be usually
loaded by the context class loader - so isn't that kind of expected anyway?
I didn't want to stuff too many implementation details into the spec.
Maybe that could be an API note however.
Would anyone be able to suggest
Thanks Stanimir,
On 9/12/14 11:24 AM, Stanimir Simeonoff wrote:
On leaks. The c-tor of LogManager leaks the thread group and the current
AccessControlContext till JVM shutdown. The constructor code should be
something like the one below with Cleaner getting ThreadGroup in its
c-tor as well.
On 09/12/2014 12:45 PM, Daniel Fuchs wrote:
However the listeners are to be invoked in the same order they have been
added.
I am still unconvinced this is worth the additional
complexity it would bring to the implementation.
The deprecated methods were using HashMap to store listeners,
and
On 09/12/2014 11:21 AM, Alan Bateman wrote:
On 12/09/2014 08:14, Peter Levart wrote:
:
Just a question about security and delayed execution...
If at the time the configuration listener is added to the LogManager,
SecurityManager is not set, the listener will be invoked directly
even if at
Hi Peter,
I like the LHM (personal favorite data structure) approach with the
dual-purpose key.
Stanimir
On Fri, Sep 12, 2014 at 2:55 PM, Peter Levart peter.lev...@gmail.com
wrote:
On 09/12/2014 12:45 PM, Daniel Fuchs wrote:
However the listeners are to be invoked in the same order they have
On 12/09/2014 11:45, Daniel Fuchs wrote:
I am still unconvinced this is worth the additional
complexity it would bring to the implementation.
The deprecated methods were using HashMap to store listeners,
and therefore the order in which listeners were invoked was also
undefined. Nobody has ever
On 9/12/14 1:55 PM, Peter Levart wrote:
On 09/12/2014 12:45 PM, Daniel Fuchs wrote:
However the listeners are to be invoked in the same order they have been
added.
I am still unconvinced this is worth the additional
complexity it would bring to the implementation.
The deprecated methods were
On 9/12/14 2:37 PM, Alan Bateman wrote:
On 12/09/2014 11:45, Daniel Fuchs wrote:
I am still unconvinced this is worth the additional
complexity it would bring to the implementation.
The deprecated methods were using HashMap to store listeners,
and therefore the order in which listeners were
On 12/09/2014 14:38, Daniel Fuchs wrote:
Would modifying the specification of addConfigurationListener()
as follows be sufficient to make the workings of the proposed
implementation clearer?
/**
* Adds a configuration listener to be invoked each time the logging
* properties are
Thanks Alan!
I have updated the webrev with your suggestions:
http://cr.openjdk.java.net/~dfuchs/webrev_8043306/webrev.04/
-- daniel
On 9/12/14 3:54 PM, Alan Bateman wrote:
On 12/09/2014 14:38, Daniel Fuchs wrote:
Would modifying the specification of addConfigurationListener()
as follows
On 12/09/2014 15:16, Daniel Fuchs wrote:
Thanks Alan!
I have updated the webrev with your suggestions:
http://cr.openjdk.java.net/~dfuchs/webrev_8043306/webrev.04/
-- daniel
A minor suggestion for readConfiguration is that Any register
configuration listeners .. might be a bit better than
Daniel,
I suppose that the propagating uncaught exceptions to the caller was the
previous behavior of the old property change methods but it seems out of place
for the LogManager. The LogManager is a global resource so broken listener
code in web app A could suppress notifications in web app
On 9/12/14 5:39 PM, Jason Mehrens wrote:
Daniel,
I suppose that the propagating uncaught exceptions to the caller was the
previous behavior of the old property change methods but it seems out of place
for the LogManager. The LogManager is a global resource so broken listener
code in web
Daniel,
Agreed. My preference would be to:
1. Catch and report them to 'System.err'. That type of code happens in the
LogManager and exceptions in this case are not the normal condition.
2. Don't specify how uncaught exceptions are handled in the javadocs.
3. Outside of this issue, make an
On 9/10/14 1:25 PM, Alan Bateman wrote:
On 10/09/2014 10:49, Daniel Fuchs wrote:
Hi,
Please find below a proposed patch for
8043306 - Provide a replacement for the API that allowed to listen
for LogManager configuration changes
https://bugs.openjdk.java.net/browse/JDK-8043306
Hi,
Please find below a proposed patch for
8043306 - Provide a replacement for the API that allowed to listen
for LogManager configuration changes
https://bugs.openjdk.java.net/browse/JDK-8043306
Proposed Patch:
http://cr.openjdk.java.net/~dfuchs/webrev_8043306/webrev.00/
On 10/09/2014 10:49, Daniel Fuchs wrote:
Hi,
Please find below a proposed patch for
8043306 - Provide a replacement for the API that allowed to listen
for LogManager configuration changes
https://bugs.openjdk.java.net/browse/JDK-8043306
Proposed Patch:
On 9/10/14 1:25 PM, Alan Bateman wrote:
On 10/09/2014 10:49, Daniel Fuchs wrote:
Hi,
Please find below a proposed patch for
8043306 - Provide a replacement for the API that allowed to listen
for LogManager configuration changes
https://bugs.openjdk.java.net/browse/JDK-8043306
On 9/10/14 2:25 PM, Paul Sandoz wrote:
FWIW i think you could do:
removeIf(e - listener == e);
Ah ah. Interesting!
Idiomatically callback methods are often prefixed with on:
onConfigurationLoaded.
Thanks! Exactly what I was looking for :-)
I'll wait for more comments before
Daniel
I think you should be able to remove the 'other instanceof
ConfigurationListener' branch in the ConfigurationListener.equals method.
Should work the same with or without that branch.
Jason
Date: Wed, 10 Sep 2014 11:49:51 +0200
From:
On 9/10/14 4:28 PM, Jason Mehrens wrote:
Daniel
I think you should be able to remove the 'other instanceof
ConfigurationListener' branch in the ConfigurationListener.equals method.
Should work the same with or without that branch.
Oh yes. I put it there just to avoid it being flagged by
31 matches
Mail list logo
