s3:share_mode_lock: make sure share_mode_cleanup_disconnected() removes the record
authorStefan Metzmacher <metze@samba.org>
Fri, 28 Aug 2020 14:28:41 +0000 (16:28 +0200)
committerStefan Metzmacher <metze@samba.org>
Thu, 3 Sep 2020 13:34:11 +0000 (13:34 +0000)
This fixes one possible trigger for "PANIC: assert failed in get_lease_type()"
https://bugzilla.samba.org/show_bug.cgi?id=14428

This is no longer enough to remove the record:

   d->have_share_modes = false;
   d->modified = true;

Note that we can remove it completely from
share_mode_cleanup_disconnected() as
share_mode_forall_entries() already sets it
when there are no entries left.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=14428

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Volker Lendecke <vl@samba.org>
(cherry picked from commit b5c0874fd5d31e252cf9ac8b84bde5c536b1e8ef)

selftest/knownfail.d/bug-14428 [deleted file]
source3/locking/share_mode_lock.c

diff --git a/selftest/knownfail.d/bug-14428 b/selftest/knownfail.d/bug-14428
deleted file mode 100644 (file)
index e198400..0000000
+++ /dev/null
@@ -1 +0,0 @@
-^samba3.blackbox.durable_v2_delay.bug.14428
index 965c5ab188377397ddbbe6b85fadc2fc526f6a74..6588f09b285bff6b0af785747caacdc56bac345f 100644 (file)
@@ -1648,6 +1648,42 @@ static bool share_mode_find_connected_fn(
        return false;
 }
 
+static bool cleanup_disconnected_share_mode_entry_fn(
+       struct share_mode_entry *e,
+       bool *modified,
+       void *private_data)
+{
+       struct cleanup_disconnected_state *state = private_data;
+       struct share_mode_data *d = state->lck->data;
+
+       bool disconnected;
+
+       disconnected = server_id_is_disconnected(&e->pid);
+       if (!disconnected) {
+               struct file_id_buf tmp1;
+               struct server_id_buf tmp2;
+               DBG_ERR("file (file-id='%s', servicepath='%s', "
+                       "base_name='%s%s%s') "
+                       "is used by server %s ==> internal error\n",
+                       file_id_str_buf(d->id, &tmp1),
+                       d->servicepath,
+                       d->base_name,
+                       (d->stream_name == NULL)
+                       ? "" : "', stream_name='",
+                       (d->stream_name == NULL)
+                       ? "" : d->stream_name,
+                       server_id_str_buf(e->pid, &tmp2));
+               smb_panic(__location__);
+       }
+
+       /*
+        * Setting e->stale = true is
+        * the indication to delete the entry.
+        */
+       e->stale = true;
+       return false;
+}
+
 bool share_mode_cleanup_disconnected(struct file_id fid,
                                     uint64_t open_persistent_id)
 {
@@ -1728,8 +1764,24 @@ bool share_mode_cleanup_disconnected(struct file_id fid,
                  ? "" : data->stream_name,
                  open_persistent_id);
 
-       data->have_share_modes = false;
-       data->modified = true;
+       ok = share_mode_forall_entries(
+               state.lck, cleanup_disconnected_share_mode_entry_fn, &state);
+       if (!ok) {
+               DBG_DEBUG("failed to clean up %zu entries associated "
+                         "with file (file-id='%s', servicepath='%s', "
+                         "base_name='%s%s%s') and open_persistent_id %"PRIu64" "
+                         "==> do not cleanup\n",
+                         state.num_disconnected,
+                         file_id_str_buf(fid, &idbuf),
+                         data->servicepath,
+                         data->base_name,
+                         (data->stream_name == NULL)
+                         ? "" : "', stream_name='",
+                         (data->stream_name == NULL)
+                         ? "" : data->stream_name,
+                         open_persistent_id);
+               goto done;
+       }
 
        /*
         * This is a temporary reproducer for the origin of