Restore Session Bug

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Restore Session Bug

jhellmann
Hi,

I am coming across a bug in Opticks. The System is crashing when loading a saved session due to the fact that items may not be ordered properly in the Optics Session File. If a dependent session item has not already been loaded, that item will fail and throw an error.

For example, AnimationController needs to be restored after Animation because it depends on Animation. However, AnimationController appears before Animation in the .session file.

Here are 3 approaches which could fix the problem:

Approach 1 (Quick Fix): Fix function getSessionItemsAnimation in SessionManagerImp.
Update this function so that "Animation" Item is written first and "AnimationController" is second. This will fix the dependencies issue and allow the object to load.
Pro: Quick Fix
Con: Does not address any other dependency issues that could show up.

Approach 2 (Robust Fix): Update the function restoreSessionItems in SessionManagerImp.cpp
This routine will collect a list of failed items.
The routine will make another attempt to load the items after the list of items has been traversed.
If these items had a dependency they should load the second time around.
The routine could keep looping though the failed list until either it is empty (successfully loaded all items) or the list stops getting smaller (unable to load remaining items).
If the list of items stops getting smaller then the unloaded items would be written to the message log.
Pro: Should fix any future dependency issues.
Con: Could be difficult to test.

Approach 3: Override the SessionManager with a custom manager.
Add an Opticks configuration setting to allow a different class to be used as the Session Manager (Current hardcoded to SessionManagerImp).
Extend the SessionManagerImp class with the custom class and override the function needed to fix the load issues.
Pro: Allow fixing of future issues without having to update Opticks core again.

Is this a known bug? Has it already been fixed? If not, could you give your recommendation for which approach you think would be best?

Thanks for your input,

- John
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Restore Session Bug

Trevor Clarke
Session save/restore is a bit of a mess and we run into these sorts of
issues fairly frequently. Fixes are sometimes straightforward and
sometimes introduce new dependency ordering issues. It's hard for me
to say which solution is best without looking further into the code.
I'd suggest you put an issue in jira with your suggestions and we can
work from there. Usually it involves trying the simplest fix and
running through some tests and going from there if that's not
sufficient.

On 9/11/15, jhellmann <[hidden email]> wrote:

> Hi,
>
> I am coming across a bug in Opticks. The System is crashing when loading a
> saved session due to the fact that items may not be ordered properly in the
> Optics Session File. If a dependent session item has not already been
> loaded, that item will fail and throw an error.
>
> For example, AnimationController needs to be restored after Animation
> because it depends on Animation. However, AnimationController appears
> before
> Animation in the .session file.
>
> Here are 3 approaches which could fix the problem:
>
> Approach 1 (Quick Fix): Fix function getSessionItemsAnimation in
> SessionManagerImp.
> Update this function so that "Animation" Item is written first and
> "AnimationController" is second. This will fix the dependencies issue and
> allow the object to load.
> Pro: Quick Fix
> Con: Does not address any other dependency issues that could show up.
>
> Approach 2 (Robust Fix): Update the function restoreSessionItems in
> SessionManagerImp.cpp
> This routine will collect a list of failed items.
> The routine will make another attempt to load the items after the list of
> items has been traversed.
> If these items had a dependency they should load the second time around.
> The routine could keep looping though the failed list until either it is
> empty (successfully loaded all items) or the list stops getting smaller
> (unable to load remaining items).
> If the list of items stops getting smaller then the unloaded items would be
> written to the message log.
> Pro: Should fix any future dependency issues.
> Con: Could be difficult to test.
>
> Approach 3: Override the SessionManager with a custom manager.
> Add an Opticks configuration setting to allow a different class to be used
> as the Session Manager (Current hardcoded to SessionManagerImp).
> Extend the SessionManagerImp class with the custom class and override the
> function needed to fix the load issues.
> Pro: Allow fixing of future issues without having to update Opticks core
> again.
>
> Is this a known bug? Has it already been fixed? If not, could you give your
> recommendation for which approach you think would be best?
>
> Thanks for your input,
>
> - John
>
>
>
> --
> View this message in context:
> http://opticks-devs.2021163.n4.nabble.com/Restore-Session-Bug-tp4653339.html
> Sent from the opticks-devs mailing list archive at Nabble.com.
>
> ------------------------------------------------------------------------------
> _______________________________________________
> Opticks-devs mailing list
> [hidden email]
> https://lists.sourceforge.net/lists/listinfo/opticks-devs
>


--
Trevor R.H. Clarke
Computer Science House
Rochester Institute of Technology
[hidden email]
http://www.csh.rit.edu/~retrev/

------------------------------------------------------------------------------
_______________________________________________
Opticks-devs mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/opticks-devs
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Restore Session Bug

goffena
Believe i have already created an issue for this:

When session was saved/loaded with a simple custom extension, no crash was noticed, but there were runtime bugs involving the animations/controller.
When session was saved/loaded with a complex custom extension (probably same one John is using), a crash was noted.

Bob Goffena



From: Trevor Clarke <[hidden email]>
To: [hidden email]
Sent: Friday, September 11, 2015 8:27 PM
Subject: Re: [Opticks-devs] Restore Session Bug

Session save/restore is a bit of a mess and we run into these sorts of
issues fairly frequently. Fixes are sometimes straightforward and
sometimes introduce new dependency ordering issues. It's hard for me
to say which solution is best without looking further into the code.
I'd suggest you put an issue in jira with your suggestions and we can
work from there. Usually it involves trying the simplest fix and
running through some tests and going from there if that's not
sufficient.

On 9/11/15, jhellmann <[hidden email]> wrote:

> Hi,
>
> I am coming across a bug in Opticks. The System is crashing when loading a
> saved session due to the fact that items may not be ordered properly in the
> Optics Session File. If a dependent session item has not already been
> loaded, that item will fail and throw an error.
>
> For example, AnimationController needs to be restored after Animation
> because it depends on Animation. However, AnimationController appears
> before
> Animation in the .session file.
>
> Here are 3 approaches which could fix the problem:
>
> Approach 1 (Quick Fix): Fix function getSessionItemsAnimation in
> SessionManagerImp.
> Update this function so that "Animation" Item is written first and
> "AnimationController" is second. This will fix the dependencies issue and
> allow the object to load.
> Pro: Quick Fix
> Con: Does not address any other dependency issues that could show up.
>
> Approach 2 (Robust Fix): Update the function restoreSessionItems in
> SessionManagerImp.cpp
> This routine will collect a list of failed items.
> The routine will make another attempt to load the items after the list of
> items has been traversed.
> If these items had a dependency they should load the second time around.
> The routine could keep looping though the failed list until either it is
> empty (successfully loaded all items) or the list stops getting smaller
> (unable to load remaining items).
> If the list of items stops getting smaller then the unloaded items would be
> written to the message log.
> Pro: Should fix any future dependency issues.
> Con: Could be difficult to test.
>
> Approach 3: Override the SessionManager with a custom manager.
> Add an Opticks configuration setting to allow a different class to be used
> as the Session Manager (Current hardcoded to SessionManagerImp).
> Extend the SessionManagerImp class with the custom class and override the
> function needed to fix the load issues.
> Pro: Allow fixing of future issues without having to update Opticks core
> again.
>
> Is this a known bug? Has it already been fixed? If not, could you give your
> recommendation for which approach you think would be best?
>
> Thanks for your input,
>
> - John
>
>
>
> --
> View this message in context:
> http://opticks-devs.2021163.n4.nabble.com/Restore-Session-Bug-tp4653339.html
> Sent from the opticks-devs mailing list archive at Nabble.com.
>
> ------------------------------------------------------------------------------
> _______________________________________________
> Opticks-devs mailing list
> [hidden email]
> https://lists.sourceforge.net/lists/listinfo/opticks-devs
>


--
Trevor R.H. Clarke
Computer Science House
Rochester Institute of Technology
[hidden email]
http://www.csh.rit.edu/~retrev/




------------------------------------------------------------------------------
_______________________________________________
Opticks-devs mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/opticks-devs



------------------------------------------------------------------------------

_______________________________________________
Opticks-devs mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/opticks-devs
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Restore Session Bug

jhellmann
I have coded a solution for this issue using the second approach. Can I submit this change using a pull request on github? What do you recommend?
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Restore Session Bug

tclarke
Administrator
Github is not the primary repo, it's only a mirror so we'll need to have a core dev review and merge the change. However, you should be ok submitting the patch via github and we can just save the patch and apply it to the subversion repo. Make sure there's a JIRA item associated and that the patch is linked in a comment.

________________________________________
From: jhellmann [[hidden email]]
Sent: Tuesday, September 15, 2015 10:15 AM
To: [hidden email]
Subject: Re: [Opticks-devs] Restore Session Bug

I have coded a solution for this issue using the second approach. Can I
submit this change using a pull request on github? What do you recommend?



--
View this message in context: https://urldefense.proofpoint.com/v2/url?u=http-3A__opticks-2Ddevs.2021163.n4.nabble.com_Restore-2DSession-2DBug-2Dtp4653339p4653344.html&d=AwICAg&c=jF7FvYH6t0RX1HrEjVCgHQ&r=BZAdW7eZ7BA-TVm8CsncxQ&m=m_1X8nVxnxPTRwgas4ywcbEAB9mX53kFww8mpzI4oOs&s=AUDipBu3Sz2bAoZ0WYwkrSJJKHBDuejhjKCmkkZu0QI&e=
Sent from the opticks-devs mailing list archive at Nabble.com.

------------------------------------------------------------------------------
_______________________________________________
Opticks-devs mailing list
[hidden email]
https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.sourceforge.net_lists_listinfo_opticks-2Ddevs&d=AwICAg&c=jF7FvYH6t0RX1HrEjVCgHQ&r=BZAdW7eZ7BA-TVm8CsncxQ&m=m_1X8nVxnxPTRwgas4ywcbEAB9mX53kFww8mpzI4oOs&s=eO5fbAFFfDRBM5fc5CbPYY1AWcQ5hrtP8HmqLw5jTMk&e=



This message and any enclosures are intended only for the addressee.  Please
notify the sender by email if you are not the intended recipient.  If you are
not the intended recipient, you may not use, copy, disclose, or distribute this
message or its contents or enclosures to any other person and any such actions
may be unlawful.  Ball reserves the right to monitor and review all messages
and enclosures sent to or from this email address.

------------------------------------------------------------------------------
_______________________________________________
Opticks-devs mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/opticks-devs
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Restore Session Bug

jhellmann
I submitted the pull request: https://github.com/tclarke/opticks/pull/2

JIRA Issue: OPTICKS-1616 https://issues.opticks.org/jira/browse/OPTICKS-1616

Thanks for your help!
Loading...