From 292afdba59de411f9d6d4ff69d11b1da068df5da Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Mon, 20 Mar 2006 23:40:43 +0000 Subject: r14596: Fix a logic bug with multiple oplock contention. 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 | 51 +++++++++++++++++++++++++++++++++------------------ 1 file changed, 33 insertions(+), 18 deletions(-) diff --git a/source3/smbd/open.c b/source3/smbd/open.c index 8913db8380..0cf8b68c28 100644 --- a/source3/smbd/open.c +++ b/source3/smbd/open.c @@ -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; -- cgit