On Wed, 2005-12-21 at 21:09 -0600, Curt Arnold wrote:
>
> > - Locking/threading/synchronization issues
>
> I think it will be difficult or impossible to safely address the
> "locks when trying to log during a toString() call" without
> potentially breaking somebody else. It is an undesirable
> established
> limitation of log4j, but it has existed there for a long time.
The problem in 1.3 is in the AppenderSkeleton class. Have you examined
the proposed Appender implementations in bug 24159, which would allow
users to avoid deadlocks in 1.3 by reconfiguring their class?
>From the bug:
------- Additional Comment #24 From Elias Ross 2005-12-04 03:56
> My unsubstantiated opinion is that the proposed solution would
> break the locking contract so it can't be addressed in a 1.x branch.
Again, the locking contract only exists for AppenderSkeleton.
Note: Although 99.99% of the users use strings or generally Object.toString()
as their output message, there is nothing to prevent users from using any other
custom object as well. For example, I wrote an Appender a while back that would
accept a data structure which contained (essentially) multiple database columns'
worth of data.
Note 2: Log4j would cause deadlocks for JBoss when a user logged an Object
which triggered a class loading operation. This is because JBoss itself uses
Log4J.
Note 3: There is a way with 1.3 to have Log4j render "printf"-style messages.
Currently, Log4J converts it to a string before calling "append", though the
string may or may not be actually logged if a filter is present. (My particular
application uses filtering based on MDC values, I would greatly benefit from
*not* rendering complicated data structures or calculated values from
Object.toString().)
An optimization of this behavior would be to create temporary object like this:
class FormattedMessage {
String msg;
Object param[];
public String toString() { /* format here */ }
}
If the call to FormattedMessage.toString() was called when holding a lock on
AppenderSkeleton, it would greatly increase deadlock potential for statements
such as:
Logger.debug("Object stuff: {}", obj);
Where "obj" itself called Logger.
------- Additional Comment #24 From Elias Ross 2005-12-04 03:56
> Another approach would be to use a thread with a timeout to call toString().
> If
> the call blocked, then some message like "Message unavailable timeout while
> calling CLASSNAME.toString() " could be created.
I don't know if there would be any way to "undo" a deadlock condition. Perhaps
you could call "Thread.stop" on one of the deadlocked threads. This has
potential to create additional deadlocks.
> Much of
> the log4j internals aren't thread safe by themselves and depend on the outer
> synchronization to keep things safe.
Many of the "internals" are public classes which are not really internal at all
and need proper documentation and/or locking.
It is irresponsible to not state or establish the contract or convention of how
a library's public classes can (or cannot) used within multiple threads. Even
though, for example, a "Layout" implementation may not be typically used outside
of a locking context or appender, it is not guaranteed or even implied this is
how it will be used.
The JDK used to be somewhat haphazard about this, but it's important to state at
least if a class is thread-safe or not thread-safe. It's also important to
state how locking is performed in the thread-safe case (or not necessary due to
design.)
Of course, users generally expect things to work for their multi-threaded
applications. It's usually good to design with this in mind. If
multi-threaded, unlocked use is dangerous, it needs to be spelled out. (Did you
know that if multiple threads modify and access a java.util.HashMap you can
lock-up one thread? If the user reads the JavaDoc they anticipate bad things
might happen without locking.)
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
