[logback-dev] [Bug 100] Serialization of objectArray in LoggingEvent does not always work

[bugzilla-daemon at pixie.qos.ch]

http://bugzilla.qos.ch/show_bug.cgi?id=100


joern at huxhorn.de changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |REOPENED
         Resolution|FIXED                       |




------- Comment #12 from joern at huxhorn.de  2008-03-13 04:00 -------
With "won't really solve the original problem" I meant that the array wouldn't
serialize correctly in case of large Strings. And since the bug report says
"Serialization of objectArray in LoggingEvent does not always work", and this
was still true, it didn't really solve the problem. ;)

I should've probably written "but introduces a different one instead" instead
of "but introduces a whole new one instead".

Sorry if this was confusing.

However, I don't think that capping the parameters is acceptable.

This is documented nowhere, neither in slf4j (where such a behavior would need
to be documented) nor in Logback.

I agree that it's probably arguable if log-messages/arguments of log-messages >
0xFFFF characters are reasonable or desirable but this should be up to the user
and not the logging framework to decide.
It sometimes *is* desirable to dump an xml structure that's larger than 64k and
this really isn't absurd in case of log.debug or log.trace.

That's why I reopened this bug.

I'd also like to know what's wrong with my patch because I really do my best to
adhere to your coding style, create minimal impact changes, add tests, etc.

- It's less complex because it's using serialization instead of writing the
array manually.
- Changing the arguments to string is done only once and only if necessary.
Serializing an event multiple times will not perform this operation again.
- It works for Strings of any size because they are serialized instead of being
written using writeUTF.
- It creates the formattedMessage lazily if needed, therefore saving time
during construction, increasing performance of the whole logging system.
- Additionally, formattedMessage should be transient and would be recreated
lazily from the message and the arguments after deserialization if needed.
I didn't declare formattedMessage transient in my patch because I didn't want
to change the serialized structure (which you just did anyway). Instead, I set
formattedMessage to null in writeObject which has a comparable effect, i.e. it
won't transfer duplicate data that can be derived from already available data
(Message + Argument + MessageFormatter) instead.
- it's completely transparent for classes using LoggingEvent itself because
  - the serialized data does not change it's form.
  - the lazy initialization of formattedMessage is invisible. It's simply not
relevant, from the callers perspective, if formattedMessage is initialized in
the c'tor or in the getFormattedMessage method.

Your change, on the other hand
- changes the serialized data without changing the serialVersionUID which
produces strange errors while trying to deserialize old events.
- creates capped strings if the Strings are to long, which results in the
creation of an *additional* String that has a size of 131070 bytes (0xFFFF
chars)!! This puts unnecessary burden on the garbage collection and is, in
fact, slower than the previous version.
- This is done again every time an event is serialized!
- By capping the arguments a serialized and deserialized event simply isn't
equal to the originally serialized event anymore, which should normally be the
case.

Is there any place where arguments are used as something else than Strings? I
really can't see any other potential problem of my patch...


-- 
Configure bugmail: http://bugzilla.qos.ch/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.