OPENDJ-1706 Persistit: online import fails with NPE

Jean-Noel Rouvignac

23.10.2014 2ac5818538b5755834cc872356fda708c2edaa6f

OPENDJ-1706 Persistit: online import fails with NPE

Code review: Matthew Swift


Problem happened when disabling the backend before online import.
Before applying the change, the config backend was iterating over the list of listeners for this configuration.
It is important to note that the list was a CopyOnWriteArrayList, and we used its iterator.
When disabling the backend, the first listener is the backend itself. This ends up calling pluggable.BackendImpl.finalizeBackend() which calls RootContainer.close(), EntryContainer.close(), AttributeIndex.close(), etc.
The net effect is that the RootContainer, EntryContainer, and AttributeIndex deregister themselves as change listeners of that config.
In the end, before the first iteration (on the backend), there are 9 config change listeners to notify. After the first loop iteration, there is only one config change listener remaining on the list of listeners: it is actually the backend we just iterated over!
However, remember the loop is performed on the CopyOnWriteArrayList iterator, which was initilized when the list had 9 elements. So it keeps notifying listeners which are no longer interested in the change.
The code then end up trying to update the indexes, by opening a transaction on the database. Problem is that the database has been closed by the call to pluggable.BackendImpl.finalizeBackend() => and BOOM! NullPointerException!

The fix consists in verifying that each listener to be notified is still interested in the config change; i.e. did it deregister itself from listening?


ConfigFileHandler.java:
In addEntry(), replaceEntry(), deleteEntry(), always verify the listener is still interested in the config change before notifying it.

 opendj-sdk/opendj3-server-dev/src/server/org/opends/server/extensions/ConfigFileHandler.java

@@ -995,12 +995,15 @@

      // Notify all the add listeners that the entry has been added.
      final ConfigChangeResult aggregatedResult = new ConfigChangeResult();
      for (ConfigAddListener l : addListeners)
      for (ConfigAddListener l : addListeners) // This is an iterator over a COWArrayList
      {
        if (addListeners.contains(l))
        { // ignore listeners that deregistered themselves
        final ConfigChangeResult result = l.applyConfigurationAdd(newEntry);
        aggregate(aggregatedResult, result);
        handleConfigChangeResult(result, newEntry.getDN(), l.getClass().getName(), "applyConfigurationAdd");
      }
      }

      throwIfUnsuccessful(aggregatedResult, ERR_CONFIG_FILE_ADD_APPLY_FAILED);
    }
@@ -1095,12 +1098,15 @@

      // Notify all the delete listeners that the entry has been removed.
      final ConfigChangeResult aggregatedResult = new ConfigChangeResult();
      for (ConfigDeleteListener l : deleteListeners)
      for (ConfigDeleteListener l : deleteListeners) // This is an iterator over a COWArrayList
      {
        if (deleteListeners.contains(l))
        { // ignore listeners that deregistered themselves
        final ConfigChangeResult result = l.applyConfigurationDelete(entry);
        aggregate(aggregatedResult, result);
        handleConfigChangeResult(result, entry.getDN(), l.getClass().getName(), "applyConfigurationDelete");
      }
      }

      throwIfUnsuccessful(aggregatedResult, ERR_CONFIG_FILE_DELETE_APPLY_FAILED);
    }
@@ -1208,12 +1214,15 @@

      // Notify all the change listeners of the update.
      final ConfigChangeResult aggregatedResult = new ConfigChangeResult();
      for (ConfigChangeListener l : changeListeners)
      for (ConfigChangeListener l : changeListeners) // This is an iterator over a COWArrayList
      {
        if (changeListeners.contains(l))
        { // ignore listeners that deregistered themselves
        final ConfigChangeResult result = l.applyConfigurationChange(currentEntry);
        aggregate(aggregatedResult, result);
        handleConfigChangeResult(result, currentEntry.getDN(), l.getClass().getName(), "applyConfigurationChange");
      }
      }

      throwIfUnsuccessful(aggregatedResult, ERR_CONFIG_FILE_MODIFY_APPLY_FAILED);
    }
@@ -1379,8 +1388,6 @@
    return null;
  }



  /**
   * Performs a subtree search starting at the provided base entry, returning
   * all entries anywhere in that subtree that match the provided filter.