At 06:11 AM 12/28/2004, Curt Arnold wrote:

Also, Item 48 of Effective Java (Synchronize access to shared mutable
data) uses an example which appears to be almost exactly the scenario
in LoggingEvent and goes to great length why the code may not perform
as expected.  Basically each thread is free to maintain its own copy of
sequenceCount and is only required to synchronize its values at thread
synchronization.

Thanks for pointing this out. sequenceCount is a static long. Just the 'long' part mandates synchronization. The fact that is shared (static) is another reason. Anyway, thanks again. A fix has been committed.



The sequenceNumber+timestamp combination makes it easy to implement
equals() method for LoggingEvent.


The implementation might be easy, but appears problematic.  The default
implementation of equals and getHashCode would be preferred if all you
cared about was object identity.

Object identity is not enough. Chainsaw as well as other receivers may access a source of LoggingEvent multiple times. In that case, we need an efficient way to discard duplicate LoggingEvents.

If you really care about value
equality, you are saying that only the class type, timestamp and
sequence number are significant.  I could abuse the current
implementation of equals by creating a LoggingEvent, cloning it (which
could be done using serialization if need be) and changing the level
and message of the clone, original.equals(clone) would still return
true since the timestamp and sequence number are still identical.


A malevolent adversary could easily fool the current equals
method. However, that would only cause a non-equal event as being seen
as equal, hence discarded as a duplicate. Not a very threatening
attack if you ask me. More importantly, log4j does not pretend to deal
with malevolent adversaries. Nevertheless, I agree with you that we
should not leave ourselves wide-open for attack either.


Also, without the synchronization issue on sequenceCount resolved,
totally unrelated logging events from different threads may compare
equal since they might have the same timestamp and sequence number.

The synchronization issue on sequenceCount should now be resolved.

Is there a particular need to implement value comparison of logging
events?  There don't appear to be any unit tests that depend on it.
Deleting the equals and getHashCode implementation didn't make any test
cases fail.  The coverage analysis indicated that LoggingEvent.equals
was called 300386 times, but every one returned true since (this ==
rObject) was true, but I didn't track down where LoggingEvent.equals
was called.

The equals method comes into play when a receiver reads a source of logging events multiple times. It's also useful in making selective SQL queries.


-- Ceki G�lc�

  The complete log4j manual: http://www.qos.ch/log4j/



---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]



Reply via email to