[Feature] Add race condition protection against hs_helper restarts

diff --git a/src/hs_helper.c b/src/hs_helper.c
index f4f00cf0540f3fd650cb95bd151a69a811f99042..cba2ca230bc39a79f06d3d9dc5ebc3cfd36387da 100644 (file)
--- a/src/hs_helper.c
+++ b/src/hs_helper.c
@@ -123,6 +123,7 @@ rspamd_hs_helper_cleanup_dir (struct hs_helper_ctx *ctx, gboolean forced)
        gint rc;
        gchar *pattern;
        gboolean ret = TRUE;
+       pid_t our_pid = getpid ();
 
        if (stat (ctx->hs_dir, &st) == -1) {
                msg_err ("cannot stat path %s, %s",
@@ -160,10 +161,38 @@ rspamd_hs_helper_cleanup_dir (struct hs_helper_ctx *ctx, gboolean forced)
        rspamd_snprintf (pattern, len, "%s%c%s", ctx->hs_dir, G_DIR_SEPARATOR, "*.hs.new");
        if ((rc = glob (pattern, 0, NULL, &globbuf)) == 0) {
                for (i = 0; i < globbuf.gl_pathc; i++) {
-                       if (unlink (globbuf.gl_pathv[i]) == -1) {
-                               msg_err ("cannot unlink %s: %s", globbuf.gl_pathv[i],
-                                               strerror (errno));
-                               ret = FALSE;
+                       /* Check if we have a pid in the filename */
+                       const gchar *end_num = globbuf.gl_pathv[i] +
+                                       strlen (globbuf.gl_pathv[i]) - (sizeof (".hs.new") - 1);
+                       const gchar *p = end_num - 1;
+                       pid_t foreign_pid = -1;
+
+                       while (p > globbuf.gl_pathv[i]) {
+                               if (g_ascii_isdigit (*p)) {
+                                       p --;
+                               }
+                               else {
+                                       p ++;
+                                       break;
+                               }
+                       }
+
+                       gulong ul;
+                       if (p < end_num && rspamd_strtoul (p, end_num - p, &ul)) {
+                               foreign_pid = ul;
+                       }
+
+                       /*
+                        * Remove only files that was left by us or some non-existing process
+                        * There could be another race condition but it would just leave
+                        * extra files which is relatively innocent?
+                        */
+                       if (foreign_pid == -1 || foreign_pid == our_pid || kill (foreign_pid, 0) == -1) {
+                               if (unlink(globbuf.gl_pathv[i]) == -1) {
+                                       msg_err ("cannot unlink %s: %s", globbuf.gl_pathv[i],
+                                                       strerror(errno));
+                                       ret = FALSE;
+                               }
                        }
                }
        }

diff --git a/src/libserver/re_cache.c b/src/libserver/re_cache.c
index b3326bcff87e79f6bac9fbf778ac15a8110a60ec..1d2f6522fb132ff9bba0cf64dd4d5124c7e2b90f 100644 (file)
--- a/src/libserver/re_cache.c
+++ b/src/libserver/re_cache.c
@@ -1894,6 +1894,7 @@ rspamd_re_cache_compile_timer_cb (EV_P_ ev_timer *w, int revents )
        struct iovec iov[7];
        struct rspamd_re_cache *cache;
        GError *err;
+       pid_t our_pid = getpid ();
 
        cache = cbdata->cache;
 
@@ -1946,8 +1947,8 @@ rspamd_re_cache_compile_timer_cb (EV_P_ ev_timer *w, int revents )
                return;
        }
 
-       rspamd_snprintf (path, sizeof (path), "%s%c%s.hs.new", cbdata->cache_dir,
-                       G_DIR_SEPARATOR, re_class->hash);
+       rspamd_snprintf (path, sizeof (path), "%s%c%s.%P.hs.new", cbdata->cache_dir,
+                       G_DIR_SEPARATOR, re_class->hash, our_pid);
        fd = open (path, O_CREAT|O_TRUNC|O_EXCL|O_WRONLY, 00600);
 
        if (fd == -1) {
@@ -2185,7 +2186,7 @@ rspamd_re_cache_compile_timer_cb (EV_P_ ev_timer *w, int revents )
                g_free (hs_flags);
 
                /* Now rename temporary file to the new .hs file */
-               rspamd_snprintf (npath, sizeof (path), "%s%c%s.hs", cbdata->cache_dir,
+               rspamd_snprintf (npath, sizeof (npath), "%s%c%s.hs", cbdata->cache_dir,
                                G_DIR_SEPARATOR, re_class->hash);
 
                if (rename (path, npath) == -1) {