Menu

#263 Use PureJavaReflectionProvider for Default Values During Session Deserialization

next release
closed
FR 102
BR 265
5
2017-07-04
2017-02-17
No

We use XSteam for Session deserialization (see class: edu.udo.scaffoldhunter.model.XMLSerialization). By default XStream uses the SunUnsafeReflectionProvider. This provider does not execute the constructor of a class and therefore does not know anything about the initial values of attributes (It just uses the java defaults). If we extend the session data (i.e. adding a variable to a config file), we have no control about the default value of config values when loading old sessions (which do not have stored that value).

For this reason, we should have a closer look at the PureJavaReflectionProvider. This provider does execute the constructor of a class and therefore does know about default values. However it has some limitations which are listed here.

A quick test revealed that exchanging the line

        xstream = new XStream(driver);

with

        xstream = new XStream(new PureJavaReflectionProvider(), driver);

leads to unloadable sessions. Thus, as a first step, we should create a list of sessions parts that conflict with the PureJavaReflectionProvider.

Discussion

  • Philipp Mewes

    Philipp Mewes - 2017-02-21
    • status: open --> in-progress
     
  • Philipp Mewes

    Philipp Mewes - 2017-02-21

    I've created this list, by solving the issues step by step. The solution is notated inside the brackets. Unfortunately the third issue occurs inside an external framwork and therefore there seems not to be a solution. It looks like that the program tries to deserialize an ImmutableList which is null.

    1. Missing default-constructor in edu-udo.scaffoldhunter.gui.ViewPosition (add Constructor)
    2. PropertyChangeSupport in edu.udo.scaffoldhunter.view.scaffoldtree.config.ConfigMapping cannot be created (make transient)
    3. NullPointerException at com.google.common.collect.ImmutableList$SerializedForm.readResolve: call to copyOf() with null (cannot be edited)
     

    Last edit: Till Schäfer 2017-03-03
  • Till Schäfer

    Till Schäfer - 2017-03-03
    • can you elaborate (2)?
    • where is (3) referenced?
     
  • Philipp Mewes

    Philipp Mewes - 2017-03-08

    (2): There seems to be a problem with PropertyChangeSupport. It crashes with the following exception: Caused by: com.thoughtworks.xstream.converters.reflection.ObjectAccessException: Cannot create java.beans.PropertyChangeSupport by JDK serialization : null Could not find the reason so far. Mark the variable as transient skips the problem.
    (3) The NullPointerException occurs at com.google.common.collect.ImmutableList at line 289. The copyOf()-call from line 592 commits null.

     
  • Till Schäfer

    Till Schäfer - 2017-03-21

    (1) I think it is better to just implement Serializable. (better fits the semantics).
    (2) The Exception should be thrown because PropertyChangeSupport does not have a default constructor and does not implement Serializable. Since the listeners have to register themself after session restoring in any way, it seems to be save to mark the variable transient. However, please be aware, that a transient variable is normaly deserialized with a null value in Java. A null, variable will create NPEs whenever calling a listener related method. Please check if the null initialization is also used by the xtream implementation (i guess it is). In this case (and only in this case), we will have to implement readObject:

    private void readObject(ObjectInputStream in) throws IOException, ClassNotFoundException {
        in.defaultReadObject();     
        listeners = new PropertyChangeSupport(this);
    

    (3) That was not my question. Where is that ImmutableList used? I.e. what Object in the session does reference it?

     
  • Till Schäfer

    Till Schäfer - 2017-03-21
    • Depends On: --> BR 265
     
  • Till Schäfer

    Till Schäfer - 2017-03-21

    i will also upgrade xstream (since it has problems with deserialization under java 7)
    -> see Bug 265

    can you please re-check the issues with newer xstream?

     
  • Till Schäfer

    Till Schäfer - 2017-03-21

    if you use the newer xstream 1.4.9 please make sure that you save a new with the new xstream 1.4.9 before applying changes here. the xstream upgrade does break the sesssion compatibility.

     
  • Philipp Mewes

    Philipp Mewes - 2017-04-03

    Tested it with the new xstream-version at branch bug-265:
    (1) still occurs but can be fixed easily by making the class Serializable
    (2) seems do be fixed
    (3) is still present. The ImmutableList that causes the bug is used in edu.udo.scaffoldhunter.gui.ViewExternalState. The member sideBarItemsExpanded is of this type.

     
  • Philipp Mewes

    Philipp Mewes - 2017-06-19

    (3) Managed to fix this issue by replacing the ImmutableList by a normal List. Returning this list with Collections.unmodiableList should also work as intended.
    After this fix (2) re-occured. The PropertyChangeSupport of ConfigMapping cannot be created. The message ist now: Cannot create type by JDK serialization.

     
  • Philipp Mewes

    Philipp Mewes - 2017-06-20

    (2) Marking the variable as transient fixes this issue. And there is no problem with NullPointerExceptions. The listeners-variable is initialized with the default-value. This also worked with the current version of XStream. Committed the changes here: [5b85a8]

     

    Related

    Commit: [5b85a8]

  • Philipp Mewes

    Philipp Mewes - 2017-06-20
    • status: in-progress --> needs-review
     
  • Till Schäfer

    Till Schäfer - 2017-06-29

    did you tested this?

    i get the folowing exception after storing and loading a session:

     

    Last edit: Till Schäfer 2017-06-29
  • Till Schäfer

    Till Schäfer - 2017-06-29
    • status: needs-review --> re-opened
     
  • Philipp Mewes

    Philipp Mewes - 2017-06-30
    • status: re-opened --> in-progress
     
  • Philipp Mewes

    Philipp Mewes - 2017-06-30

    Yes, i saved a complete session and could restore it without errors. How did this exception occur? Could not reproduce it.
    However i detected a bug with the PropertyChangeSupport in ConfigMapping. If the mapping is configured via the MappingDialog, the session is saved and loaded again, ConfigMapping.listeners is null. Adding a default-constructor, which initializes this variable, seems to be executed during the session loading, but the variable remains null.

     
  • Till Schäfer

    Till Schäfer - 2017-06-30

    i have done some work in every view, e.g. started a clustering in the dendrogram and heatmap view. I have also selected some molecules, configered a background color for notdes in the cloud view and so on.

    Testing should be done with as many features used as possible. The exceptions are only triggered if something is actually stored in the session.

     

    Last edit: Till Schäfer 2017-06-30
  • Philipp Mewes

    Philipp Mewes - 2017-07-03

    Ok, re-tested it with more features. Although i could not reproduce the exception so far, i think it is related to the same problem, described above. The exception seems to be caused indirectly by a NullPointerException at ConfigMapping.setMinimumPropertyValue(). In the respective line the listeners-variable is accessed and seems to be null.

    EDIT: You described a solution for the listeners-variable beeing null, by using readObject(). I will check if this solves the issue.

     

    Last edit: Philipp Mewes 2017-07-03
  • Philipp Mewes

    Philipp Mewes - 2017-07-03

    Implementing readObjects() solved the problem, but breaks the session-compatibility ([e0887a]).

     

    Related

    Commit: [e0887a]

  • Philipp Mewes

    Philipp Mewes - 2017-07-04

    After testing each view with all available features, no exceptions occured. Please review, if everything works.

     
  • Philipp Mewes

    Philipp Mewes - 2017-07-04
    • status: in-progress --> needs-review
     
  • Till Schäfer

    Till Schäfer - 2017-07-04

    works as expected now. thx.

     
  • Till Schäfer

    Till Schäfer - 2017-07-04
    • status: needs-review --> closed
    • Group: any future version --> next release
     
  • Till Schäfer

    Till Schäfer - 2017-07-04

    merged: [d78dbc]

     

    Related

    Commit: [d78dbc]


Log in to post a comment.