Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Delay initialization of the UserPreferencesManager. #4005

Merged
merged 2 commits into from
Aug 29, 2017
Merged

Delay initialization of the UserPreferencesManager. #4005

merged 2 commits into from
Aug 29, 2017

Conversation

rhwood
Copy link
Contributor

@rhwood rhwood commented Aug 28, 2017

Allow the InstanceManager to initialize the default instance of the UserPreferencesManager after it has created the default instance.

Fixes #3999.

The problem here was that, in the absence of per-profile/per-computer stored user preferences, and in the presence of the older-style per-profile/all-computers stored user preferences, creating and initializing the default instance before registering it was causing the infinite looping. The solution was to implement the InstanceManagerAutoInitialize interface for JmriUserPreferencesManager and to move its initialization into the initialize method(). This allows the InstanceManager to register and create the default and then initialize it.

…it has automatically created the default instance.
@rhwood rhwood added the Bug label Aug 28, 2017
@rhwood rhwood added this to the 4.9.4 milestone Aug 28, 2017
@rhwood
Copy link
Contributor Author

rhwood commented Aug 29, 2017

@bobjacobsen Does this look like an acceptable fix to #3999?

@bobjacobsen
Copy link
Member

This looks good, both from a code point reading and from running it with my updated #3963 that starts PanelPro with a profile that currently hangs.

I think it's ready to be merged.

One question: If we have multiple sets (old and new) present, do we really want to create both in memory? Or should the older one be ignored and not loaded? Or am I misunderstanding what's happening here?

@rhwood
Copy link
Contributor Author

rhwood commented Aug 29, 2017

One question: If we have multiple sets (old and new) present, do we really want to create both in memory? Or should the older one be ignored and not loaded? Or am I misunderstanding what's happening here?

Assuming a "set" is stored user preferences, the logic is:

  • If newer set exists, use it, ignoring any possible older set.
  • If newer set does not exist, but older set does, read older set to populate newer set.
  • If neither set exists, create a new set and use it.
    It is entirely possible that the middle path creates, uses, and unnecessarily retains objects in memory during that one session, but I don't think it does (I have not done a memory analysis on this).

Or am I misunderstanding the question?

@bobjacobsen
Copy link
Member

Or am I misunderstanding the question?

No, that was a nice answer, quite on point.

In some deep way, I don't have an understanding of how the preference system works within the profile-tree system. I had gone looking for some indication of how those interacted without success. Your description above is certainly a good way to handle it, I just couldn't see that's what it was doing.

@rhwood rhwood merged commit 87af353 into JMRI:master Aug 29, 2017
@rhwood rhwood deleted the looping-configure-manager branch August 29, 2017 10:49
rhwood added a commit to JMRI/website that referenced this pull request Aug 31, 2017
@rhwood rhwood self-assigned this Aug 31, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants