You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by Simon Kitching <sk...@apache.org> on 2006/02/21 05:59:51 UTC

[logging] Avalon log not Serializable?

Hi,

I've been checking whether all the standard Log adapter classes are
Serializable. The Avalon logger declares itself to be Serializable, but
my reading of the code indicates this won't work. Can someone
confirm/deny this?

The javadoc has this:
 * <strong>Note:</strong> <code>AvalonLogger</code> implements
 * <code>Serializable</code> for reasons of consistency and backwards
compatibility. 
 * However, serializable is not recommended.

Here's the implementation:

    /** Avalon logger used to perform log */
    private transient Logger logger = null;

    public Logger getLogger() {
        return logger;
    }

    public boolean isDebugEnabled() {
        return getLogger().isDebugEnabled();
    }


When an object is deserialized, member "logger" will end up as null.
Note that this is because it gets the *default* value for that type;
instance member initialisers are not run. A call to isDebugEnabled will
then result in a NullPointerException.

There is no readObject or readResolve method to customise serialization
behaviour.


The Log4JLogger class appears to work correctly, because it implements
getLogger like this:
    public Logger getLogger() {
        if (logger == null) {
            logger = Logger.getLogger(name);
        }
        return (this.logger);
    }
which reinitialises the logger member after deserialization. Personally
I would like to see a readObject method do this instead as all other
methods could then access the logger member directly and avoid a method
call; however both will work fine. Hmm..perhaps this method call is the
cause of the slowdown in Log4JLogger since 1.2 revealed by Robert's
recent tests?

Perhaps we should implement AvalonLogger.getLogger like this?
    public Logger getLogger() {
        if (logger == null) {
            if (defaultLogger != null) {
              logger = defaultLogger.getChildLogger(name);
            } else {
              throw new NotSerializableException(
                "AvalonLogger is only deserializable when" +
                 "AvalonLogger.setDefaultLogger has been called");
            }
        }

        return logger;
    }



All the other loggers appear ok:
 * Jdk14Logger uses the same approach as Log4JLogger.
 * LogKitLogger does the same.
 * SimpleLog doesn't have an "underlying Log object" to handle;
   it just needs to serialize its own primitive config members
   which looks like it will work fine.
 * NoOpLog doesn't have any problem (nothing to serialize).


Cheers,

Simon


---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


Re: [logging] Avalon log not Serializable?

Posted by Craig McClanahan <cr...@apache.org>.
On 2/20/06, Simon Kitching <sk...@apache.org> wrote:
>
> Hi,
>
> I've been checking whether all the standard Log adapter classes are
> Serializable. The Avalon logger declares itself to be Serializable, but
> my reading of the code indicates this won't work. Can someone
> confirm/deny this?


I suspect you are correct that it won't work, and would need to be modified
along the lines of the other loggers to recreate transient instances where
needed.

If they're not there already, test cases that use a log instance, then
serialize and deserialize it, then try to use it again would catch things
like this and avoid the potential for future regressions.

Craig

Re: [logging] Avalon log not Serializable?

Posted by robert burrell donkin <ro...@blueyonder.co.uk>.
On Tue, 2006-02-21 at 17:59 +1300, Simon Kitching wrote:

> The Log4JLogger class appears to work correctly, because it implements
> getLogger like this:
>     public Logger getLogger() {
>         if (logger == null) {
>             logger = Logger.getLogger(name);
>         }
>         return (this.logger);
>     }
> which reinitialises the logger member after deserialization. Personally
> I would like to see a readObject method do this instead as all other
> methods could then access the logger member directly and avoid a method
> call; however both will work fine. Hmm..perhaps this method call is the
> cause of the slowdown in Log4JLogger since 1.2 revealed by Robert's
> recent tests?

i've tracked this down now. there appears to be big performance
difference between calling category and logger when the log system is
not configured. add a basic, no logging configuration brought the
figures all into line.

so, the good news is that we're about as fast as the other JCL
releases. 

FWIW i also prefer the readObject solution

- robert


---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


Re: [logging] Avalon log not Serializable?

Posted by robert burrell donkin <ro...@blueyonder.co.uk>.
On Tue, 2006-02-21 at 23:58 +1300, Simon Kitching wrote:

> Craig Mcc has also confirmed it looks broken to him.

yep, broke

> Here are the options I can see:
> (a) remove AvalonLogger from the JCL 1.1 distribution.
> (b) remove Serializable marker interface from AvalonLogger
> (c) on deserialize, try to handle things properly (we can under
> some circumstances). If not possible, create a dummy Logger
> object that just discards all output.

<snip>

> My preference in order would be: B, A, C. 
> All seem feasable.
> 
> Comments?

B, C, A :)

> BTW, I have checked for unit tests for Serializable behaviour. Log4j,
> Simple and Jdk14 all have a unit test checking that a
> serialized/deserialized instance works ok.
> 
> NoOp, LogKit and AvalonLogger do not have serialization unit tests. As
> NoOp has *no* instance members of any sort a test doesn't seem
> vital :-). I'm willing to create a serialization test for LogKit.

if it's not going to take too much time, that'd be good. please use the
JCL1 branch.

- robert


---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


Re: [logging] Avalon log not Serializable?

Posted by Torsten Curdt <tc...@apache.org>.
>
> These two issues *can* be sort-of worked around by my proposed option
> (c). However I don't really like the idea of silently disabling  
> logging.

Me neither :-/

> In short, I believe that Avalon logging in general, and the JCL
> AvalonLogger in particular are not designed to handle serialization  
> and
> so we just shouldn't be trying to support that.

Probably right

cheers
--
Torsten

Re: [logging] Avalon log not Serializable?

Posted by Simon Kitching <sk...@apache.org>.
On Thu, 2006-02-23 at 13:07 +1100, Torsten Curdt wrote:
> > Here are the options I can see:
> > (a) remove AvalonLogger from the JCL 1.1 distribution.
> 
> Naaaa ...bad idea IMO. We would have to deprecate it first.

Ok.

> 
> > (b) remove Serializable marker interface from AvalonLogger
> 
> It's probably the best to fix it in the Avalon code.
> 
> It's old, it's becoming reviled just because it's Avalon
> ...but still some people are using it ...and some people
> still have commit rights to the old codebase :)

Are *all* avalon logger implementations required to implement
Serializable? If so, then we *can* indeed fix this simply by declaring
the underlying Logger object non-transient. However I have no idea how
this could be checked, who could be asked, etc. Personally I doubt that
any such requirement exists.

Assuming that some Avalon loggers are *not* serializable then there are
two problems.

The first is that the AvalonLogger class in JCL has a constructor that
takes an avalon Logger object directly. In that case we don't even have
a category name; we therefore have no way at all to obtain an equivalent
underlying Logger object on deserialize.

The second is that the AvalonLogger constructor that takes a category
name has to assume that someone has called static
AvalonLogger.setDefaultLogger. This is pretty poor design for a start;
if they haven't then there is little option but to throw an exception
which is not good style for a logging library. The same problem occurs
on deserialize; we do have a category name but if the static
setDefaultLogger method hasn't been called then there's no way to obtain
an underlying Logger.

These two issues *can* be sort-of worked around by my proposed option
(c). However I don't really like the idea of silently disabling logging.

In short, I believe that Avalon logging in general, and the JCL
AvalonLogger in particular are not designed to handle serialization and
so we just shouldn't be trying to support that.

Cheers,

Simon



---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


Re: [logging] Avalon log not Serializable?

Posted by Torsten Curdt <tc...@apache.org>.
> Here are the options I can see:
> (a) remove AvalonLogger from the JCL 1.1 distribution.

Naaaa ...bad idea IMO. We would have to deprecate it first.

> (b) remove Serializable marker interface from AvalonLogger

It's probably the best to fix it in the Avalon code.

It's old, it's becoming reviled just because it's Avalon
...but still some people are using it ...and some people
still have commit rights to the old codebase :)

cheers
--
Torsten


Re: [logging] Avalon log not Serializable?

Posted by Simon Kitching <sk...@apache.org>.
On Tue, 2006-02-21 at 17:59 +1300, Simon Kitching wrote:
> Hi,
> 
> I've been checking whether all the standard Log adapter classes are
> Serializable. The Avalon logger declares itself to be Serializable, but
> my reading of the code indicates this won't work. Can someone
> confirm/deny this?

I've done some more work, and it sure looks like AvalonLogger is broken
for Serialization. It's had quite an interesting history.

Craig Mcc has also confirmed it looks broken to him.

Here are the options I can see:
(a) remove AvalonLogger from the JCL 1.1 distribution.
(b) remove Serializable marker interface from AvalonLogger
(c) on deserialize, try to handle things properly (we can under
some circumstances). If not possible, create a dummy Logger
object that just discards all output.

AvalonLogger was only added in the 1.0.4 release (and therefore
obviously was not *deprecated*). However the avalon project is
completely dead AFAIK. One potential issue is that the latest "stable"
release of Apache FOP is v0.20 which still uses avalon-framework. While
old, I expect Apache FOP is used in quite a few apps. And apps using
Apache FOP *might* redirect JCL via the AvalonLogger adapter to the
avalon logging system. Crazy but possible.

Option b is also reasonable to me. Serialization never worked for
AvalonLogger; any attempt to use a deserialized object would immediately
result in a NullPointerException. By implication, there are no systems
out there using serialization for AvalonLogger objects. And the
existence of the Serializable marker is not a compile-time
incompatibility (except in truly perverse code). So surely dropping the
marker won't do any harm. It does mean we can't claim "all standard log
adapters are Serializable" but I think we can live with that.

Option c doesn't look too hard; the avalon Logger interface is almost
exactly like the JCL Log interface so we could whip up a dummy class
like NoOpLog which we can create an instance of.


My preference in order would be: B, A, C. 
All seem feasable.

Comments?



BTW, I have checked for unit tests for Serializable behaviour. Log4j,
Simple and Jdk14 all have a unit test checking that a
serialized/deserialized instance works ok.

NoOp, LogKit and AvalonLogger do not have serialization unit tests. As
NoOp has *no* instance members of any sort a test doesn't seem
vital :-). I'm willing to create a serialization test for LogKit..

Cheers,

Simon


---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org