net_nntp: Add log for expired articles and allow interrupting expiration.

* Add articles to expire log when removed from history.
* Treat articles no longer in the spool as "expired" when rebuilding
  history.
* Abort expiration if unload is pending.
* Run expiration in the background by default, so that
  net_nntp can be unloaded while expiration is running.
  The tests still run in the foreground for better synchronization
  (and since we're not expiring large numbers of articles in the tests).

Author: InterLinked1

nets/net_nntp.c

@@ -90,7 +90,7 @@ static int nntps_port = DEFAULT_NNTPS_PORT;
 static int nnsp_port = DEFAULT_NNSP_PORT;
 
 static int nntp_enabled = 1, nntps_enabled = 1, nnsp_enabled = 1;
-int nntp_unloading = 0; /* Used extern by nntp_feed_nntp.c */
+int nntp_unloading = 0; /* Used extern by nntp_feed_nntp.c and nntp_history.c */
 void *thismodule; /* Used extern by nntp_suck.c */
 
 static bbs_rwlock_t nntp_lock;
@@ -2153,17 +2153,52 @@ static int cli_delarticle(struct bbs_cli_args *a)
 	return 0;
 }
 
+static pthread_t expire_thread;
+static int expire_done = 0;
+
+static void *do_expiration(void *varg)
+{
+	char *group = varg;
+	int res = history_expire(group); /* OK if group is NULL */
+	free_if(group);
+	if (res < 0) {
+		bbs_warning("Failed to expire articles\n");
+	} else {
+		bbs_verb(5, "Expired %d news article%s\n", res, ESS(res));
+	}
+	expire_done = 1;
+	return NULL;
+}
+
 static int cli_expire(struct bbs_cli_args *a)
 {
 	const char *group = a->argv[2];
-	int res;
+	char *varg = group ? strdup(group) : NULL;
+	if (expire_thread) {
+		if (!expire_done) {
+			bbs_dprintf(a->fdout, "Expiration already in progress\n");
+			return -1;
+		}
+		bbs_pthread_join(expire_thread, NULL);
+	}
+	if (bbs_pthread_create(&expire_thread, NULL, do_expiration, varg)) {
+		free_if(varg);
+		bbs_dprintf(a->fdout, "Failed to expire articles\n");
+		return -1;
+	}
+	bbs_dprintf(a->fdout, "Started expiration task\n");
+	return 0;
+}
 
-	res = history_expire(group); /* OK if group is NULL */
+static int cli_fgexpire(struct bbs_cli_args *a)
+{
+	const char *group = a->argv[2];
+	int res = history_expire(group); /* OK if group is NULL */
 	if (res < 0) {
 		bbs_dprintf(a->fdout, "Failed to expire articles\n");
 		return -1;
 	}
-	bbs_dprintf(a->fdout, "Expired %d article%s\n", res, ESS(res));
+	bbs_dprintf(a->fdout, "Expired %d news article%s\n", res, ESS(res));
 	return 0;
 }
 
@@ -4815,6 +4850,7 @@ static struct bbs_cli_entry cli_commands_nntp[] = {
 	BBS_CLI_COMMAND(cli_setstatus, "news setstatus", 4, "Edit posting status for a newsgroup", "news setstatus <group> <y/n/m>"),
 	BBS_CLI_COMMAND(cli_delarticle, "news delarticle", 4, "Delete an article", "news delarticle <group> <article number>"),
 	BBS_CLI_COMMAND(cli_expire, "news expire", 2, "Remove expired articles from the spool (optionally just for one group)", "news expire <group>"),
+	BBS_CLI_COMMAND(cli_fgexpire, "news fgexpire", 2, "Remove expired articles from the spool (optionally just for one group), in foreground", "news expire <group>"),
 	BBS_CLI_COMMAND(cli_feedflush, "news feedflush", 2, "Flush queued articles for feed(s)", "news feedflush [<site>]"),
 	BBS_CLI_COMMAND(cli_feedstats, "news feedstats", 2, "Show outgoing feed stats", "news feedstats [<site>]"),
 };
@@ -5135,6 +5171,9 @@ static void cleanup_subsystems(void)
 {
 	active_cleanup();
 	spool_cleanup();
+	if (expire_thread) {
+		bbs_pthread_join(expire_thread, NULL);
+	}
 	history_cleanup();
 }
 

nets/net_nntp/nntp.h

@@ -513,6 +513,15 @@ int spool_article_create(struct article_groups *groups, struct article_info *art
 
 int spool_article_delete_by_number(const char *groupname, int article_num);
 
+/*!
+ * \brief Whether a given article exists in a group
+ * \param groupname
+ * \param article_num
+ * \retval 1 if article exists in the spool
+ * \retval 0 if article doesn't exist in the spool
+ */
+int spool_article_exists(const char *groupname, int article_num);
+
 /*!
  * \brief Whether any message with this Message-ID exists in any group
  * \param messageid Message-ID of article, if searching by message-ID

nets/net_nntp/nntp_history.c

@@ -28,6 +28,7 @@
 /* #define DEBUG_EXPIRE */
 
 extern int min_history;
+extern int nntp_unloading;
 
 /* Each thread opens a group's overview file for reading/writing, so multiple readers can operate simultaneously */
 static bbs_mutex_t histlock;
@@ -150,6 +151,11 @@ void history_cleanup(void)
 		fclose(histfp);
 		histfp = NULL;
 	}
+
+	/* Lock and unlock to ensure that all expire operations have finished */
+	bbs_mutex_lock(&histlock);
+	bbs_mutex_unlock(&histlock);
+
 	bbs_mutex_destroy(&histlock);
 	RWLIST_WRLOCK_REMOVE_ALL(&retention_patterns, entry, free);
 }
@@ -261,12 +267,16 @@ static inline int should_expire_article(const char *group, time_t now, time_t ar
 
 int history_expire(const char *pattern)
 {
-	FILE *newfp;
+	FILE *newfp, *expirefp;
 	char buf[NNTP_BUFSIZ];
 	int total_removed = 0, links_removed = 0;
 	int line = 0;
+	int error = 0;
 	time_t history_cutoff, now;
 	char template[TMPNAME_BUFSIZ];
+	char expirelogfile[NNTP_MAX_PATH_LENGTH];
+
+	snprintf(expirelogfile, sizeof(expirelogfile), "%s/%s", bbs_log_dir(), "nntp_expire.log");
 
 	RWLIST_RDLOCK(&retention_patterns);
 	if (!any_retention_patterns_remove_articles()) {
@@ -277,7 +287,12 @@ int history_expire(const char *pattern)
 	RWLIST_UNLOCK(&retention_patterns); /* REQUIRE_HISTORY_FP returns, so we make sure we're unlocked there */
 
 	if (bbs_mutex_trylock(&histlock)) {
-		bbs_warning("An expiration operation is already in-progress, rejecting concurrent expiration attempt\n");
+		bbs_notice("An expiration operation is already in-progress, rejecting concurrent expiration attempt\n");
+		return -1;
+	}
+
+	if (nntp_unloading) {
+		bbs_notice("Module unload is pending, not starting article expiration\n");
 		return -1;
 	}
 
@@ -292,6 +307,16 @@ int history_expire(const char *pattern)
 	history_cutoff = now - (86400 * min_history);
 
 	REQUIRE_HISTORY_FP(histfp);
+
+	expirefp = fopen(expirelogfile, "a");
+	if (!expirefp) {
+		bbs_error("Failed to append to %s: %s\n", expirelogfile, strerror(errno));
+		fclose(newfp);
+		unlink(template);
+		bbs_mutex_unlock(&histlock);
+		return -1;
+	}
+
 	REWIND_HISTORY(histfp);
 
 	RWLIST_RDLOCK(&retention_patterns);
@@ -304,8 +329,11 @@ int history_expire(const char *pattern)
 		char links[4 * NNTP_MAX_PATH_LENGTH] = "";
 		char *linkspos = links;
 		size_t linksleft = sizeof(links);
-		char *msgid = strsep(&restofline, "\t");
+		char *msgid;
+
 		line++;
+
+		msgid = strsep(&restofline, "\t");
 		if (unlikely(!msgid)) {
 			bbs_warning("History file %s corrupted (line %d)\n", history_file, line);
 			continue;
@@ -350,7 +378,8 @@ int history_expire(const char *pattern)
 
 				if (should_expire_article(grp, now, arrival_time, expires)) {
 					int res = expire_article(grp, artnum);
-					if (!res) {
+					/* If we successfully expired it, or the article is no longer present in the spool, treat as expired */
+					if (!res || !spool_article_exists(grp, artnum)) {
 						keep = 0;
 						links_removed++;
 					}
@@ -372,16 +401,42 @@ int history_expire(const char *pattern)
 		/* Keep in history if article still exists in any groups, or if its age is less than min_history days (even if article may no longer be in any groups) */
 		if (groups_kept || (min_history && (arrival_time > history_cutoff))) {
 			fprintf(newfp, "%s\t%ld~%s~%s\t%s\n", msgid, arrival_time, expires_str, bytes_str, links);
+		} else {
+			/* If article is now being completely removed from history, add it to the expire log, the final vestige of this article's transient existence! */
+			fprintf(expirefp, "%s\t%ld~%s~%s\n", msgid, arrival_time, expires_str, bytes_str);
 		}
 		if (!groups_kept || groups_removed) {
 			EXPIRE_DEBUG(4, "%s %s (kept: %d, removed: %d)\n", !groups_kept && groups_removed > 0 ? "Permanently deleted" : groups_removed ? "Partially deleted" : "Keeping", msgid, groups_kept, groups_removed);
 		}
+
+		/* If we are unloading, skip all further expiration checks and just copy the remainder of the original history file to the new one.
+		 * This way we can stop ~immediately if finishing expiration would take a long time. */
+		if (nntp_unloading) {
+			long startoffset, histsize;
+
+			bbs_debug(4, "Aborting remaining expiration checks\n");
+
+			if (!total_removed) {
+				/* No changes have been made yet, we can just keep the original file */
+				break;
+			}
+
+			startoffset = ftell(histfp);
+			fseek(histfp, 0, SEEK_END);
+			histsize = ftell(histfp);
+			fflush(newfp);
+			if (bbs_copy_file(fileno(histfp), fileno(newfp), (int) startoffset, (int) (histsize - startoffset)) < 0) { /* Copy original headers, not including empty line */
+				error = 1; /* If we failed to append the remainder of the file, don't discard the original history file */
+			}
+			break;
+		}
 	}
 	RWLIST_UNLOCK(&retention_patterns);
 
 	/* If needed, swap in the new history file and update pointers */
+	fclose(expirefp);
 	fclose(newfp);
-	if (total_removed > 0) {
+	if (total_removed > 0 && !error) {
 		bbs_verb(5, "Swapping in new history file (removed %d link%s, completely removed %d article%s)\n", links_removed, ESS(links_removed), total_removed, ESS(total_removed));
 		fclose(histfp);
 		if (rename(template, history_file)) {

nets/net_nntp/nntp_spool.c

@@ -80,6 +80,11 @@ int spool_article_delete_by_number(const char *groupname, int article_num)
 	return tradspool_article_delete_by_number(groupname, article_num);
 }
 
+int spool_article_exists(const char *groupname, int article_num)
+{
+	return tradspool_article_exists(groupname, article_num);
+}
+
 int spool_article_stat(struct nntp_session *nntp, const char *messageid, const char *groupname, int article_num)
 {
 	return tradspool_article_stat(nntp, messageid, groupname, article_num);

nets/net_nntp/nntp_spool_trad.c

@@ -1040,6 +1040,15 @@ int tradspool_article_delete_by_number(const char *groupname, int article_num)
 	return 0;
 }
 
+int tradspool_article_exists(const char *groupname, int article_num)
+{
+	char artpath[NNTP_MAX_PATH_LENGTH];
+	if (!build_group_article_path(groupname, article_num, artpath, sizeof(artpath))) {
+		return 1;
+	}
+	return 0;
+}
+
 int tradspool_article_stat(struct nntp_session *nntp, const char *messageid, const char *groupname, int article_num)
 {
 	if (messageid) {

nets/net_nntp/nntp_spool_trad.h

@@ -29,5 +29,6 @@ int tradspool_group_overview_header(struct nntp_session *nntp, const char *field
 int tradspool_overview_header_list(struct nntp_session *nntp, enum list_category listcat, const char *argument);
 int tradspool_article_create(struct article_groups *groups, struct article_info *artinfo, int srcfd, size_t len);
 int tradspool_article_delete_by_number(const char *groupname, int article_num);
+int tradspool_article_exists(const char *groupname, int article_num);
 int tradspool_article_stat(struct nntp_session *nntp, const char *messageid, const char *groupname, int article_num);
 int tradspool_article_send(struct nntp_session *nntp, enum article_part_filter filter, const char *messageid, const char *groupname, int article_num);

tests/test_nntp_transit.c

@@ -266,7 +266,7 @@ static int run(void)
 		"Expires: Thu, 31 Dec 2099 23:59:59 +00:00\r\n"
 	);
 	GROUP_EXPECT(client1, "test.expires", 1, 3, 3);
-	TEST_CLI_COMMAND("news expire");
+	TEST_CLI_COMMAND("news fgexpire");
 	GROUP_EXPECT(client1, "test.expires", EMPTY_LOW_WATERMARK(4), EMPTY_HIGH_WATERMARK(3), 0); /* All articles should have been removed */
 
 	/* Should refuse article that was deleted since it was recent enough */
@@ -278,14 +278,14 @@ static int run(void)
 		"Expires: Thu, 31 Dec 2099 23:59:59 +00:00\r\n"
 	);
 	GROUP_EXPECT(client1, "test.expires2", 1, 3, 3);
-	TEST_CLI_COMMAND("news expire");
+	TEST_CLI_COMMAND("news fgexpire");
 	GROUP_EXPECT(client1, "test.expires2", 1, 3, 3); /* No articles should have expired */
 
 	TAKETHIS_ADDITIONAL(peer1, "<expires2.4@" TEST_HOSTNAME ">", TEST_EMAIL_EXTERNAL, "test.expires2",
 		"Expires: Fri, 31 Dec 1999 23:59:59 +00:00\r\n"
 	);
 	GROUP_EXPECT(client1, "test.expires2", 1, 4, 4);
-	TEST_CLI_COMMAND("news expire");
+	TEST_CLI_COMMAND("news fgexpire");
 	GROUP_EXPECT(client1, "test.expires2", 1, 3, 3); /* Article 4 was deleted due to its Expires header being in the past */
 
 	SWRITE(client1, "ARTICLE <expires2.2@" TEST_HOSTNAME ">\r\n");