Created attachment 8646 [details]
A patch for the new test

I will submit a possible, but inadequate patch soon.

This patch fixes the problem as far as I can see, however, it does not fully mimic the behavior of Windows. Windows sends a STATUS_PENDING result, while this does not. Also, the line numbers might be off a bit:

--- samba-3.6.12.orig/source3/smbd/smb2_create.c 2012-06-24 10:21:16.000000000 -0700
+++ samba-3.6.12/source3/smbd/smb2_create.c      2013-03-14 01:00:44.575614048 -0700
@@ -244,7 +244,18 @@ NTSTATUS smbd_smb2_request_process_creat
        }
        tevent_req_set_callback(tsubreq, smbd_smb2_request_create_done, smb2req);

-       return smbd_smb2_request_pending_queue(smb2req, tsubreq);
+       /*
+        * If the open was deferred, then do not call request pending queue
+        * because that means an OpLock break has occurred and we want to
+        * wait.
+        */
+       if (smb2req->smb1req && open_was_deferred_smb2(smb2req->smb1req->sconn,
+                                               smb2req->smb1req->mid)) {
+               DEBUG(10, ("Deferred request (OpLock Break) ... doing nothing\n"));
+               return NT_STATUS_OK;
+       }
+       else
+               return smbd_smb2_request_pending_queue(smb2req, tsubreq);
 }

Created attachment 8647 [details]
A slightly updated test

This adds the test to the list of tests as well.

Currently, in smbd_smb2_request_pending_queue we do the following:

        if (req->in.vector_count > idx + SMBD_SMB2_NUM_IOV_PER_REQ) {
                /*
                 * We're trying to go async in a compound
                 * request chain. This is not allowed.
                 * Cancel the outstanding request.
                 */
                bool ok = tevent_req_cancel(req->subreq);
                if (ok) {
                        return NT_STATUS_OK;
                }
                TALLOC_FREE(req->subreq);
                return smbd_smb2_request_error(req,
                        NT_STATUS_INTERNAL_ERROR);
        }

However, if we have sent an OpLock Break, then we do want to go async and send a pending response if we want to do the same as Windows.

Perhaps the simplest way is to add a flag field to the request stating that we need to go async. Don't we have one of those already?

Created attachment 8684 [details]
A better patch that has seen more testing

This is a better patch that has seen more testing.

We do need to allow going async in a compound request if we want to handle this case. Also, as a result, only re-arrange the request if the command causing us to go async is not the first.

I should also have a better test soon as well. One that cleans up after itself.

Sorry for the delay Richard, I'm working on this now !

Jeremy.

Created attachment 8844 [details]
Raw patch for master.

Ok, just to show you where I'm going with this (it's a work in progress :-) here is a raw patch that contains both a slightly modified version of your torture test and a complete fix for master that allows it to pass the new smb2.compound-break test !

It breaks the interim2 test (returns NT_STATUS_CANCELLED - should be NT_STATUS_INTERNAL_ERROR) but it should show how I'm going to fix this.

I'll update further when I've gotten everything back working and broken into a set of micro-patches for master/4.0.x.

The nice thing is this should be pretty simple to back-port to 3.6.x, and addresses the issue fully as per the SMB2 spec + Windows implementation notes.

Cheers,

Jeremy.

Ah. The interim2 is a knownfail !

knownfail:^samba3.smb2.compound.interim2 # wrong return code (STATUS_CANCELLED)

That means the raw patch is actually correct...

I'll break it up into a set of micro-changes then submit to master.

Jeremy.

Created attachment 8846 [details]
git-am fix for master and 4.0.next.

Ok, I think this is what we need as a separate set of patches. It includes your torture test Richard (listed with you as the author). Once I've gotten everything passing tests I'll submit on the mailing list.

Jeremy.

Comment on attachment 8846 [details]
git-am fix for master and 4.0.next.

Bah, uploaded wrong file - sorry :-(.

Created attachment 8847 [details]
git-am fix for master and 4.0.next.

Ok, here is the real git-am patchset :-).

Jeremy.

Created attachment 8869 [details]
git-am patchset for 4.0.x

Code that was pushed to master (contains cherry-pick info).

Jeremy.

Re-assigning to Karolin for inclusion in 4.0.next.

(Patch already reviewed by 2 engineers, jra + realrichardsharpe).

Pushed to autobuild-v4-0-test.

Re-assigning to me to get the 3.6.next patchset done :-).

Created attachment 8873 [details]
git-am patchset for 3.6.next

Backport of the 4.0.x fix for 3.6.

Richard and Ira - please check !

Jeremy.

Comment on attachment 8873 [details]
git-am patchset for 3.6.next

Looks good to me.

Re-assigning back to Karolin so she can add the last patch to 3.6.next.

(Than we can finally close the thing :-).

Jeremy.

Pushed to v3-6-test.
Closing out bug report.

Thanks!