Felix
  1. Felix
  2. FELIX-609

PreferencesService might return invalid Preferences object for a user

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: prefs-1.0.2
    • Fix Version/s: prefs-1.0.4
    • Component/s: Preferences Service
    • Labels:
      None

      Description

      When a Preferences object has been obtained for a given user, it can be removed using removeNode(). A following call to getUserPreferences(...) will then return the old, now invalid, Preferences object, leading to an IllegalStateException when trying to add new data to the node. From this moment on, it is not possible to obtain a valid Preferences object for this user.

      The spec does not provide a definite answer about this case; 106.4 comes closest with "getUserPreferences(String) - Return a Preferences object associated with the user name that is given as argument. If the user does not exist, a new root is created atomically." Intuitively, one might expect that removeNode() restores the PreferencesService to the same state it was before getUserPreferences(...) was called.

      The invalid Preferences object is caused by caching behavior in PreferencesServicesImpl's getUserPreferences(...), which checks whether a user's root node has already been created; if so, it will be returned, if not, it will be created.

      91 PreferencesImpl result = (PreferencesImpl) this.trees.get(name);
      92 // if the tree does not exist yet, create it
      93 if (result == null)

      { 94 result = new PreferencesImpl(new PreferencesDescription(this.bundleId, name), this.storeManager); 95 this.trees.put(name, result); 96 }

      The most plausible solution is to add an extra clause to the if-statement on line 93, stating something like
      93 if (result == null || !result.isValid()) {
      provided that isValid() will then return the valid field of PreferencesImpl.

      An alternative would be to detect the removal of a node which does not have a parent (is a root node), and then remove it from the PreferencesServiceImpl's trees; this is not that complicated, but requires some extra code.

        Activity

        Angelo van der Sijpt created issue -
        Hide
        Angelo van der Sijpt added a comment -

        Typo.

        Show
        Angelo van der Sijpt added a comment - Typo.
        Angelo van der Sijpt made changes -
        Field Original Value New Value
        Description When a Preferences object has been obtained for a given user, it can be removed using removeNode(). A following call to getUserPreferences(...) will then return the old, now invalid, Preferences object, leading to an IllegalStateException when trying to add new data to the node. From this moment on, it is not possible to obtain a valid Preferences object for this user.

        The spec does not provide a definite answer about this case; 106.4 comes closest with "getUserPreferences(String) - Return a Preferences object associated with the user name that is given as argument. If the user does not exist, a new root is created atomically." Intuitively, one might expect that removeNode() restores the PreferencesService to the same state it was before getUserPreferences(...) was called.

        The invalid Preferences object is caused by caching behavior in PreferencesServicesImpl's getUserPreferences(...), which checks whether a user's root node has already been created; if so, it will be returned, if not, it will be created.

        91 PreferencesImpl result = (PreferencesImpl) this.trees.get(name);
        92 // if the tree does not exist yet, create it
        93 if (result == null) {
        94 result = new PreferencesImpl(new PreferencesDescription(this.bundleId, name), this.storeManager);
        95 this.trees.put(name, result);
        96 }

        The most plausible solution is to add an extra clause to the if-statement on line 93, stating something like
        93 if (result == null || result.isValid()) {
        provided that isValid() will then return the valid field of PreferencesImpl.

        An alternative would be to detect the removal of a node which does not have a parent (is a root node), and then remove it from the PreferencesServiceImpl's trees; this is not that complicated, but requires some extra code.
        When a Preferences object has been obtained for a given user, it can be removed using removeNode(). A following call to getUserPreferences(...) will then return the old, now invalid, Preferences object, leading to an IllegalStateException when trying to add new data to the node. From this moment on, it is not possible to obtain a valid Preferences object for this user.

        The spec does not provide a definite answer about this case; 106.4 comes closest with "getUserPreferences(String) - Return a Preferences object associated with the user name that is given as argument. If the user does not exist, a new root is created atomically." Intuitively, one might expect that removeNode() restores the PreferencesService to the same state it was before getUserPreferences(...) was called.

        The invalid Preferences object is caused by caching behavior in PreferencesServicesImpl's getUserPreferences(...), which checks whether a user's root node has already been created; if so, it will be returned, if not, it will be created.

        91 PreferencesImpl result = (PreferencesImpl) this.trees.get(name);
        92 // if the tree does not exist yet, create it
        93 if (result == null) {
        94 result = new PreferencesImpl(new PreferencesDescription(this.bundleId, name), this.storeManager);
        95 this.trees.put(name, result);
        96 }

        The most plausible solution is to add an extra clause to the if-statement on line 93, stating something like
        93 if (result == null || !result.isValid()) {
        provided that isValid() will then return the valid field of PreferencesImpl.

        An alternative would be to detect the removal of a node which does not have a parent (is a root node), and then remove it from the PreferencesServiceImpl's trees; this is not that complicated, but requires some extra code.
        Marcel Offermans made changes -
        Assignee Marcel Offermans [ marrs ]
        Hide
        Marcel Offermans added a comment -

        Someone please confirm the fix has the desired effect.

        Show
        Marcel Offermans added a comment - Someone please confirm the fix has the desired effect.
        Marcel Offermans made changes -
        Resolution Fixed [ 1 ]
        Status Open [ 1 ] Resolved [ 5 ]
        Hide
        Angelo van der Sijpt added a comment -

        It has been a while, but yes, the trunk behavior is now OK.

        Show
        Angelo van der Sijpt added a comment - It has been a while, but yes, the trunk behavior is now OK.
        Angelo van der Sijpt made changes -
        Status Resolved [ 5 ] Closed [ 6 ]
        Carsten Ziegeler made changes -
        Fix Version/s prefs-1.0.4 [ 12313951 ]
        Affects Version/s prefs-1.0.2 [ 12314050 ]

          People

          • Assignee:
            Marcel Offermans
            Reporter:
            Angelo van der Sijpt
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development