IdentityFetcher/SubscriptionManager: Fix AssertionError

Have observed this for IdentityFetcher in the wild: Daemon Thread [<noname>(17)] (Suspended (exception AssertionError)) IdentityFetcher.scheduleCommandProcessing() line: 435 IdentityFetcher.storeAbortFetchCommandWithoutCommit(Identity) line: 396 WebOfTrust.deleteWithoutCommit(Identity) line: 2192 WebOfTrust.upgradeDatabaseFormatVersion2() line: 689 WebOfTrust.upgradeDB() line: 592 WebOfTrust.runPlugin(PluginRespirator) line: 230 PluginHandler.startPlugin(PluginManager, PluginInfoWrapper) line: 44 Did not observe it for SubscriptionManager yet, but I would suspect that it can happen there as well. Thus, also fixing it there.
hyphanet · Mar 10, 2015 · a3573a9 · a3573a9
1 parent 0a23080
commit a3573a9
Show file tree

Hide file tree

Showing 2 changed files with 27 additions and 4 deletions.
diff --git a/src/plugins/WebOfTrust/IdentityFetcher.java b/src/plugins/WebOfTrust/IdentityFetcher.java
@@ -431,8 +431,18 @@ public void storeUpdateEditionHintCommandWithoutCommit(String identityID) {
 	}
 
 	private void scheduleCommandProcessing() {
-        assert(mJob != MockDelayedBackgroundJob.DEFAULT)
-            : "Should not be called before start() as mJob won't execute then!";
+	    // We do not do the following commented out assert() because:
+	    // 1) WebOfTrust.upgradeDB() can rightfully cause this function to be called before start()
+	    //    (in case it calls functions which create commands):
+	    //    upgradeDB()'s job is to update outdated databases to a new database format. No
+	    //    subsystem of WOT which accesses the database should be start()ed before the database
+	    //    has been converted to the current format, including the IdentityFetcher.
+	    // 2) It doesn't matter if we don't process commands which were created before start():
+	    //    start() will delete all old commands anyway and compute the set of identities to be
+	    //    fetched from scratch.
+	    //
+	    /* assert(mJob != MockDelayedBackgroundJob.DEFAULT)
+            : "Should not be called before start() as mJob won't execute then!"; */
 
         // We do not do this because some unit tests intentionally stop() us before they run. 
         /*  assert (!mJob.isTerminated()) : "Should not be called after stop()"; */

diff --git a/src/plugins/WebOfTrust/SubscriptionManager.java b/src/plugins/WebOfTrust/SubscriptionManager.java
@@ -1965,8 +1965,18 @@ public int getPriority() {
 	 * Schedules the {@link #run()} method to be executed after a delay of {@link #PROCESS_NOTIFICATIONS_DELAY}
 	 */
 	private void scheduleNotificationProcessing() {
-        assert(mJob != MockDelayedBackgroundJob.DEFAULT)
-            : "Should not be called before start() as mJob won't execute then!";
+        // We do not do the following commented out assert() because:
+        // 1) WebOfTrust.upgradeDB() can rightfully cause this function to be called before start()
+        //    (in case it calls functions which create Notifications):
+        //    upgradeDB()'s job is to update outdated databases to a new database format. No
+        //    subsystem of WOT which accesses the database should be start()ed before the database
+        //    has been converted to the current format, including the SubscriptionManager.
+        // 2) It doesn't matter if we don't process Notifications which were queued before start():
+        //    start() will automatically delete all existing Clients, so the Notifications would be
+        //    deleted as well.
+        //
+        /* assert(mJob != MockDelayedBackgroundJob.DEFAULT)
+            : "Should not be called before start() as mJob won't execute then!"; */
 
         // We do not do this because some unit tests intentionally stop() us before they run.
         /*  assert (!mJob.isTerminated()) : "Should not be called after stop()!"; */
@@ -2005,6 +2015,9 @@ protected synchronized void start() {
         // any Clients/Notifications can be created: Notifications to Clients will only be sent out
         // if scheduleNotificationProcssing() is functioning at the moment a Notification is
         // created.
+        // Notice: Once you change Clients/Notifications to be persistent across restarts of WOT,
+        // and therefore remove this, please make sure to notice and update the comments inside
+        // scheduleNotificationProcessing().
 		deleteAllClients();
 
 		final PluginRespirator respirator = mWoT.getPluginRespirator();