[Asterisk-code-review] pbx/pbx spool: Fix issue when call files were executed too e... (asterisk[master])

[Joshua Colp]

Joshua Colp has submitted this change and it was merged.

Change subject: pbx/pbx_spool: Fix issue when call files were executed too early
......................................................................


pbx/pbx_spool: Fix issue when call files were executed too early

pbx_spool used to delete/move the call file upon successful outgoing
call completion, but did not delete it from in-memory list of files
(dirlist, used only when compiled with inotify/kqueue support).
That resulted in an extra attempt to process that filename after
retrytime seconds.
Then, if a new file with the same name appears that is scheduled
in future further than the completed one plus its retrytime,
then it gets executed earlier than expected.

This patch fixes remove_from_queue function to also remove the entry
from the dirlist.

ASTERISK-17069 #close
Reported by: Jeremy Kister

ASTERISK-24442 #close
Reported by: tootai

Change-Id: If9ec9b88073661ce485d6b008fd0b2612e49a28b
---
M pbx/pbx_spool.c
1 file changed, 27 insertions(+), 8 deletions(-)

Approvals:
  Richard Mudgett: Looks good to me, but someone else must approve
  Joshua Colp: Looks good to me, approved; Verified



diff --git a/pbx/pbx_spool.c b/pbx/pbx_spool.c
index 0dad606..c858ed2 100644
--- a/pbx/pbx_spool.c
+++ b/pbx/pbx_spool.c
@@ -102,6 +102,14 @@
 };
 
 #if defined(HAVE_INOTIFY) || defined(HAVE_KQUEUE)
+struct direntry {
+	AST_LIST_ENTRY(direntry) list;
+	time_t mtime;
+	char name[0];
+};
+
+static AST_LIST_HEAD_STATIC(dirlist, direntry);
+
 static void queue_file(const char *filename, time_t when);
 #endif
 
@@ -323,6 +331,10 @@
 	char newfn[256];
 	const char *bname;
 
+#if defined(HAVE_INOTIFY) || defined(HAVE_KQUEUE)
+	struct direntry *cur;
+#endif
+
 	if (!ast_test_flag(&o->options, SPOOL_FLAG_ALWAYS_DELETE)) {
 		struct stat current_file_status;
 
@@ -332,6 +344,19 @@
 			}
 		}
 	}
+
+#if defined(HAVE_INOTIFY) || defined(HAVE_KQUEUE)
+	AST_LIST_LOCK(&dirlist);
+	AST_LIST_TRAVERSE_SAFE_BEGIN(&dirlist, cur, list) {
+		if (!strcmp(cur->name, o->fn)) {
+			AST_LIST_REMOVE_CURRENT(list);
+			ast_free(cur);
+			break;
+		}
+	}
+	AST_LIST_TRAVERSE_SAFE_END;
+	AST_LIST_UNLOCK(&dirlist);
+#endif
 
 	if (!ast_test_flag(&o->options, SPOOL_FLAG_ARCHIVE)) {
 		unlink(o->fn);
@@ -486,14 +511,6 @@
 	return 0;
 }
 
-#if defined(HAVE_INOTIFY) || defined(HAVE_KQUEUE)
-struct direntry {
-	AST_LIST_ENTRY(direntry) list;
-	time_t mtime;
-	char name[0];
-};
-
-static AST_LIST_HEAD_STATIC(dirlist, direntry);
 
 #if defined(HAVE_INOTIFY)
 /* Only one thread is accessing this list, so no lock is necessary */
@@ -501,6 +518,8 @@
 static AST_LIST_HEAD_NOLOCK_STATIC(openlist, direntry);
 #endif
 
+#if defined(HAVE_INOTIFY) || defined(HAVE_KQUEUE)
+
 static void queue_file(const char *filename, time_t when)
 {
 	struct stat st;

-- 
To view, visit https://gerrit.asterisk.org/432
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: If9ec9b88073661ce485d6b008fd0b2612e49a28b
Gerrit-PatchSet: 2
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Ivan Poddubny <ivan.poddubny at gmail.com>
Gerrit-Reviewer: Ashley Sanders <asanders at digium.com>
Gerrit-Reviewer: Ivan Poddubny <ivan.poddubny at gmail.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>