Re: Latest Java release causes runtime exception

[Mark Brouwer <ma...@marbro.org>]

Hi Dennis,

As I had some time to fix my own code as result of the change in the 
serialized form of j.u.l.Level I had to look a bit better what has been 
going on with j.u.l.Level and how to solve it. As a result I implemented 
a more robust solution than that currently in trunk, more about that later.

On 2/22/13 5:38 PM, Dennis Reedy wrote:

> After a couple of go arounds discussing this in Jira, I recalled what you said here, and adding support for the 'localizedLevelName' in the com.sun.jini.logging.Levels$LevelData class seems to do the trick. So my original suggestion to create a CustomLevel as a way to deal with this is being rolled back. The original approach of creating a *real* level that introduces no runtime requirements for River classes in a JVM that reads LogRecords that have either Levels.HANDLED and Levels.FAILED remains.
>

Looking in most recent source code for JDK 1.7 and 1.6 it seems quite 
clear that the field localizedLevelName should have been declared 
transient, as there is no particular need to carry it around in the 
serialized form. JDK 1.8 ea doesn't contain these changes yet, so the 
Levels class worked without problems over there.

Do you have a bug id for http://bugs.sun.com/ for this issue, because to 
prevent from duplicate reports I tried to search it. But that search 
function in Sun/Oracle bug database is severely broken for quite a while 
now.

However I have to correct my statement that Oracle breached the 
serialized contract for j.u.l.Level. If the field localizedLevelName was 
an intentional change it should have been documented with the @serial 
tag, but according to section 5.6.2 of the the serialization 
specification 
(http://docs.oracle.com/javase/7/docs/platform/serialization/spec/version.html#6678) 
adding a field is a compatible change and therefore the code in 
com.sun.jini.logging.Levels shouldn't have failed. Although mentioned as 
an incompatible change removing a field in a later version of a class 
should not break deserialization with an older version either as in such 
a case the default value is just assigned (although the state of the 
object might be broken).

The real problems lies with c.s.j.l.Levels.LevelData as the class 
c.s.j.l.Levels.ClassReplacingObjectOutputStream makes through the 
writeClassDescriptor method the ObjectOutputStream 'believe' that 
LevelData is effectively a Level instance. The class descriptor is 
therefore in case of the latest JDK 1.6. and 1.7 telling there are 4 
fields available in Level. LevelData is only able to provide 3 of these 
(reading the 4th field through reflection doesn't seem to fail). I 
analyzed the byte stream of what was serialized and although it was 
mentioned there are 4 fields only values for 3 of them were actually 
provided. Deserialization therefore failed because the class descriptor 
is really expecting 4 values in the stream while there are only 3, hence 
the EOF execption (in this particular case).

The solution is that LevelData must *really* mimic the serialized form 
of the Level class. This can be accomplished by controlling the 
serialized fields for LevelData by setting serialPersistentFields:

     /** Fields from {@link Level} that needs to be serialized. */
     private static final ObjectStreamField[] serialPersistentFields =
             ObjectStreamClass.lookup(Level.class).getFields();

By implementing the following writeObject method on LevelData, the 
values for the know fields of Level can be provides. This code will be 
able to handle further evolution of the Level class as well:

     /**
      * Controls the writing of the actual fields to mimic those
      * available in {@link Level}.
      */
     private void writeObject(java.io.ObjectOutputStream out)
         throws IOException
     {
         ObjectOutputStream.PutField putFields = out.putFields();

         for (int i = 0 ; i < serialPersistentFields.length ; i++) {
             ObjectStreamField field = serialPersistentFields[i];

             if (field.getName().equals("name")) {
                 putFields.put(field.getName(), name);
             }
             else if (field.getName().equals("resourceBundleName")) {
                 putFields.put(field.getName(), resourceBundleName);
             }
             else if (field.getName().equals("value")) {
                 putFields.put(field.getName(), value);
             }

             // if a field name is unknown it looks like the Level class
             // evolved, but we are in no position to provide a
             // meaningful value, so therefore don't provide a value as
             // the putField object already has the default values set
             // for all fields as returned by serialPersistentFields
         }

         out.writeFields();
     }

https://issues.apache.org/jira/browse/RIVER-416 contains a patch against 
the trunk that could be applied, as well as the complete source code. My 
j.u.l.Levels implementation was based on the initial submission from Sun 
to Apache so it lacks the reformatting that was done in a previous 
submit. I'm not closely monitoring what has changed with regard to the 
project conventions, but I still assume the coding style in place is the 
same as effective within Sun. I can even remember some fierce 
discussions about tabs versus spaces in the beginning but as I see there 
are still tabs and spaces that stayed the same too ;-)
IMHO maintaining the some coding style would ease in taking and giving 
of code between various codebases out there in the field.

The above solution has been tested on various JDKs, I did verify the 
actual objects created (all real instances of Level) and analyzed the 
serialized byte streams and this does really does seems to do the trick. 
At least I found it an interesting journey through the serialization 
specification ...

Regards,
-- 
Mark Brouwer

Re: Latest Java release causes runtime exception

[Dennis Reedy <de...@gmail.com>]

Mark,

Apologies for leaving an answer to your question below blank, I didn't catch it until now.

> Do you have a bug id for http://bugs.sun.com/ for this issue, because to prevent from duplicate reports I tried to search it. But that search function in Sun/Oracle bug database is severely broken for quite a while now.

No, I do not have a bug id for this, feel free to create a report.

Thanks again for the additional robustness in your patch.

Regards

Dennis

Re: Latest Java release causes runtime exception

[Dennis Reedy <de...@gmail.com>]

On Mar 12, 2013, at 1233PM, Mark Brouwer wrote:
> 
> 
> https://issues.apache.org/jira/browse/RIVER-416 contains a patch against the trunk that could be applied, as well as the complete source code. My j.u.l.Levels implementation was based on the initial submission from Sun to Apache so it lacks the reformatting that was done in a previous submit. I'm not closely monitoring what has changed with regard to the project conventions, but I still assume the coding style in place is the same as effective within Sun. I can even remember some fierce discussions about tabs versus spaces in the beginning but as I see there are still tabs and spaces that stayed the same too ;-)
> IMHO maintaining the some coding style would ease in taking and giving of code between various codebases out there in the field.

I dont agree with the maintaining the current coding style, at times I find much of the code base to be unreadable (until it is formatted). Many of the conventions just dont seem to make sense anymore. For example, line length of 80 characters. Perhaps this made sense when out display resolution was 640x780, (recall the conventions were last updated in 1999), but not for today. I dont even want to start the discussion on tabs vs spaces (I use spaces). 

> 
> The above solution has been tested on various JDKs, I did verify the actual objects created (all real instances of Level) and analyzed the serialized byte streams and this does really does seems to do the trick. At least I found it an interesting journey through the serialization specification ...

The approach I had provided also was tested on various JDKs. However, I'm glad to accept your patch

Thanks

Dennis