dbwrap_watch: Don't alert ourselves, fix raw.oplock.batch26 race
authorVolker Lendecke <vl@samba.org>
Mon, 30 Sep 2019 09:39:11 +0000 (11:39 +0200)
committerRalph Boehme <slow@samba.org>
Wed, 2 Oct 2019 08:01:41 +0000 (08:01 +0000)
commit32d6cc84cf8e0cf278b5715b8a9d66b7c0a2a6d2
tree74336630934f41e931f1630a8791ad01a48715b8
parent64da66a75c688b0443b5d5afa4f73ac51b96c504
dbwrap_watch: Don't alert ourselves, fix raw.oplock.batch26 race

This fixes the following flaky test:

UNEXPECTED(failure): samba3.raw.oplock.batch26(nt4_dc)
REASON: Exception: Exception: (../../source4/torture/raw/oplock.c:3718): wrong value for break_info.count got 0x2 - should be 0x1

You can reproduce it with two small msleeps, which means it's a race
condition:

diff --git a/source3/smbd/open.c b/source3/smbd/open.c
index 20b5a3e294c..126c7fc021d 100644
--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -1917,6 +1917,14 @@ NTSTATUS send_break_message(struct messaging_context *msg_ctx,
  DATA_BLOB blob;
  NTSTATUS status;

+ {
+ static bool sent = false;
+ if (sent) {
+ smb_msleep(500);
+ }
+ sent = true;
+ }
+
  if (DEBUGLVL(10)) {
  struct server_id_buf buf;
  DBG_DEBUG("Sending break message to %s\n",
diff --git a/source3/smbd/oplock.c b/source3/smbd/oplock.c
index b3da84b1269..d9c4dbb9487 100644
--- a/source3/smbd/oplock.c
+++ b/source3/smbd/oplock.c
@@ -858,6 +858,8 @@ static void process_oplock_break_message(struct messaging_context *msg_ctx,
  uint16_t break_to;
  bool break_needed = true;

+ smb_msleep(100);
+
  msg = talloc(talloc_tos(), struct oplock_break_message);
  if (msg == NULL) {
  DBG_WARNING("talloc failed\n");

15a8af075a2 introduced a bug where we immediately wake up ourselves
after doing a watch_send, leading to two inter-smbd oplock break
messages for this case. In theory, this should not matter, as in the
oplock break handler in the destination smbd we check

(fsp->sent_oplock_break != NO_BREAK_SENT)

so that the break does not get sent twice. However, with the above two
sleeps the oplock holding client could send out its oplock downgrade
while the second inter-smbd break messages was on its way.

The real fix would be to note in the share mode array that the
inter-smbd message has already been sent, but as other users of
dbwrap_watched_watch_send might also be affected by this bug, this fix
should be sufficient to get rid of this flaky test.

Unfortunately, dbwrap_watch.c is now pretty complex and needs some
serious refactoring to become understandable again. But that's
something for another day, sorry.

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>
source3/lib/dbwrap/dbwrap_watch.c