r14596: Fix a logic bug with multiple oplock contention.
authorJeremy Allison <jra@samba.org>
Mon, 20 Mar 2006 23:40:43 +0000 (23:40 +0000)
committerGerald (Jerry) Carter <jerry@samba.org>
Wed, 10 Oct 2007 16:15:38 +0000 (11:15 -0500)
The sad thing is the core of this bug fix is just
removing a paranoia "exit_server" call, as the
rest of the logic was already correct :-).

Lots of comments to explain the logic added.

I will look at adding tests to exercise this,
might be possible.

Jeremy.
(This used to be commit c2488db727e1a00f112be7b169de9e6208e311f3)

source3/smbd/open.c

index 8913db8380cda3a1ad8239b417ea130389639839..0cf8b68c28b5fe7bf2f39971d45d9683ce94a766 100644 (file)
@@ -1091,20 +1091,14 @@ files_struct *open_file_ntcreate(connection_struct *conn,
                struct deferred_open_record *state =
                        (struct deferred_open_record *)pml->private_data.data;
 
+               /* Remember the absolute time of the original
+                  request with this mid. We'll use it later to
+                  see if this has timed out. */
+
                request_time = pml->request_time;
                delayed_for_oplocks = state->delayed_for_oplocks;
 
-               /* There could be a race condition where the dev/inode pair
-                  has changed since we deferred the message. If so, just
-                  remove the deferred open entry and return sharing
-                  violation. */
-
-               /* If the timeout value is non-zero, we need to just return
-                  sharing violation. Don't retry the open as we were not
-                  notified of a close and we don't want to trigger another
-                  spurious oplock break. */
-
-               /* Now remove the deferred open entry under lock. */
+               /* Remove the deferred open entry under lock. */
                lck = get_share_mode_lock(NULL, state->dev, state->inode, NULL, NULL);
                if (lck == NULL) {
                        DEBUG(0, ("could not get share mode lock\n"));
@@ -1327,15 +1321,16 @@ files_struct *open_file_ntcreate(connection_struct *conn,
 
                if (delay_for_oplocks(lck, fsp)) {
                        struct deferred_open_record state;
-                       struct timeval timeout;
 
-                       if (delayed_for_oplocks) {
-                               DEBUG(0, ("Trying to delay for oplocks "
-                                         "twice\n"));
-                               exit_server("exiting");
-                       }
+                       /* This is a relative time, added to the absolute
+                          request_time value to get the absolute timeout time.
+                          Note that if this is the second or greater time we enter
+                          this codepath for this particular request mid then
+                          request_time is left as the absolute time of the *first*
+                          time this request mid was processed. This is what allows
+                          the request to eventually time out. */
 
-                       timeout = timeval_set(OPLOCK_BREAK_TIMEOUT*2, 0);
+                       struct timeval timeout;
 
                        /* Normally the smbd we asked should respond within
                         * OPLOCK_BREAK_TIMEOUT seconds regardless of whether
@@ -1343,6 +1338,13 @@ files_struct *open_file_ntcreate(connection_struct *conn,
                         * measure here in case the other smbd is stuck
                         * somewhere else. */
 
+                       timeout = timeval_set(OPLOCK_BREAK_TIMEOUT*2, 0);
+
+                       /* Nothing actually uses state.delayed_for_oplocks
+                          but it's handy to differentiate in debug messages
+                          between a 30 second delay due to oplock break, and
+                          a 1 second delay for share mode conflicts. */
+
                        state.delayed_for_oplocks = True;
                        state.dev = dev;
                        state.inode = inode;
@@ -1434,8 +1436,21 @@ files_struct *open_file_ntcreate(connection_struct *conn,
                                struct timeval timeout;
                                struct deferred_open_record state;
 
+                               /* This is a relative time, added to the absolute
+                                  request_time value to get the absolute timeout time.
+                                  Note that if this is the second or greater time we enter
+                                  this codepath for this particular request mid then
+                                  request_time is left as the absolute time of the *first*
+                                  time this request mid was processed. This is what allows
+                                  the request to eventually time out. */
+
                                timeout = timeval_set(0, SHARING_VIOLATION_USEC_WAIT);
 
+                               /* Nothing actually uses state.delayed_for_oplocks
+                                  but it's handy to differentiate in debug messages
+                                  between a 30 second delay due to oplock break, and
+                                  a 1 second delay for share mode conflicts. */
+
                                state.delayed_for_oplocks = False;
                                state.dev = dev;
                                state.inode = inode;