The branch, master has been updated
       via  ebc1579 torture3: Reproducer for bug 10284
       via  6b6920b smbd: Fix bug 10284
      from  a793ac0 smbd: Pull mtime handling into open_file_ntcreate

http://gitweb.samba.org/?p=samba.git;a=shortlog;h=master


- Log -----------------------------------------------------------------
commit ebc157961aa910d84b31e9f7a1b4c99267294999
Author: Volker Lendecke <[email protected]>
Date:   Thu Nov 21 16:16:33 2013 +0100

    torture3: Reproducer for bug 10284
    
    Signed-off-by: Volker Lendecke <[email protected]>
    Reviewed-by: Stefan Metzmacher <[email protected]>
    
    Autobuild-User(master): Volker Lendecke <[email protected]>
    Autobuild-Date(master): Tue Nov 26 22:53:04 CET 2013 on sn-devel-104

commit 6b6920b02905661ae661a894e3bd8d2c744d7003
Author: Volker Lendecke <[email protected]>
Date:   Thu Nov 21 21:05:29 2013 +0100

    smbd: Fix bug 10284
    
    If we msg_read_send on a nonempty channel, we create one
    tevent_immediate. If we directly receive another message and from
    within the msg_read_send's tevent_req callback we immediately do
    another msg_read_send, we end up with two tevent_immediate events for
    msg_channel_trigger with just one incoming message. Test to follow.
    
    This patch simplifies msg_channel.c by removing the explicit immediate
    events. Instead, it relies on the implicit immediate event available
    via tevent_req_defer_callback. For messages received from tdb with
    a msg_read_send req pending, we directly finish that request without
    putting the message on the queue.
    
    Bug: https://bugzilla.samba.org/show_bug.cgi?id=10284
    Signed-off-by: Volker Lendecke <[email protected]>
    Reviewed-by: Stefan Metzmacher <[email protected]>

-----------------------------------------------------------------------

Summary of changes:
 source3/lib/msg_channel.c  |  100 ++++++++++++++------------------------------
 source3/selftest/tests.py  |    1 +
 source3/torture/proto.h    |    1 +
 source3/torture/test_msg.c |   86 +++++++++++++++++++++++++++++++++++++
 source3/torture/torture.c  |    1 +
 5 files changed, 120 insertions(+), 69 deletions(-)


Changeset truncated at 500 lines:

diff --git a/source3/lib/msg_channel.c b/source3/lib/msg_channel.c
index 625d07c..8e23fd4 100644
--- a/source3/lib/msg_channel.c
+++ b/source3/lib/msg_channel.c
@@ -41,9 +41,6 @@ static void msg_channel_init_got_ctdb(struct tevent_req 
*subreq);
 static void msg_channel_init_got_msg(struct messaging_context *msg,
                               void *priv, uint32_t msg_type,
                               struct server_id server_id, DATA_BLOB *data);
-static void msg_channel_trigger(struct tevent_context *ev,
-                               struct tevent_immediate *im,
-                               void *priv);
 static int msg_channel_destructor(struct msg_channel *s);
 
 struct tevent_req *msg_channel_init_send(TALLOC_CTX *mem_ctx,
@@ -157,6 +154,12 @@ fail:
        return err;
 }
 
+struct msg_read_state {
+       struct tevent_context *ev;
+       struct msg_channel *channel;
+       struct messaging_rec *rec;
+};
+
 static void msg_channel_init_got_msg(struct messaging_context *msg,
                                     void *priv, uint32_t msg_type,
                                     struct server_id server_id,
@@ -167,7 +170,6 @@ static void msg_channel_init_got_msg(struct 
messaging_context *msg,
        struct messaging_rec *rec;
        struct messaging_rec **msgs;
        size_t num_msgs;
-       struct tevent_immediate *im;
 
        rec = talloc(s, struct messaging_rec);
        if (rec == NULL) {
@@ -184,6 +186,19 @@ static void msg_channel_init_got_msg(struct 
messaging_context *msg,
        }
        rec->buf.length = data->length;
 
+       if (s->pending_req != NULL) {
+               struct tevent_req *req = s->pending_req;
+               struct msg_read_state *state = tevent_req_data(
+                       req, struct msg_read_state);
+
+               s->pending_req = NULL;
+
+               state->rec = talloc_move(state, &rec);
+               tevent_req_defer_callback(req, s->ev);
+               tevent_req_done(req);
+               return;
+       }
+
        num_msgs = talloc_array_length(s->msgs);
        msgs = talloc_realloc(s, s->msgs, struct messaging_rec *, num_msgs+1);
        if (msgs == NULL) {
@@ -192,28 +207,11 @@ static void msg_channel_init_got_msg(struct 
messaging_context *msg,
        s->msgs = msgs;
        s->msgs[num_msgs] = talloc_move(s->msgs, &rec);
 
-       if (s->pending_req == NULL) {
-               return;
-       }
-
-       im = tevent_create_immediate(s);
-       if (im == NULL) {
-               goto fail;
-       }
-       tevent_schedule_immediate(im, s->ev, msg_channel_trigger, s);
        return;
 fail:
        TALLOC_FREE(rec);
 }
 
-struct msg_read_state {
-       struct tevent_context *ev;
-       struct tevent_req *req;
-       struct msg_channel *channel;
-       struct messaging_rec *rec;
-};
-
-static int msg_read_state_destructor(struct msg_read_state *s);
 static void msg_read_got_ctdb(struct tevent_req *subreq);
 
 struct tevent_req *msg_read_send(TALLOC_CTX *mem_ctx,
@@ -221,7 +219,6 @@ struct tevent_req *msg_read_send(TALLOC_CTX *mem_ctx,
                                 struct msg_channel *channel)
 {
        struct tevent_req *req;
-       struct tevent_immediate *im;
        struct msg_read_state *state;
        void *msg_tdb_event;
        size_t num_msgs;
@@ -231,28 +228,28 @@ struct tevent_req *msg_read_send(TALLOC_CTX *mem_ctx,
                return NULL;
        }
        state->ev = ev;
-       state->req = req;
        state->channel = channel;
 
        if (channel->pending_req != NULL) {
                tevent_req_error(req, EBUSY);
                return tevent_req_post(req, ev);
        }
-       channel->pending_req = req;
-       channel->ev = ev;
-       talloc_set_destructor(state, msg_read_state_destructor);
 
        num_msgs = talloc_array_length(channel->msgs);
        if (num_msgs != 0) {
-               im = tevent_create_immediate(channel);
-               if (tevent_req_nomem(im, req)) {
-                       return tevent_req_post(req, ev);
-               }
-               tevent_schedule_immediate(im, channel->ev, msg_channel_trigger,
-                                         channel);
-               return req;
+               state->rec = talloc_move(state, &channel->msgs[0]);
+               memmove(channel->msgs, channel->msgs+1,
+                       sizeof(struct messaging_rec *) * (num_msgs-1));
+               channel->msgs = talloc_realloc(
+                       channel, channel->msgs, struct messaging_rec *,
+                       num_msgs - 1);
+               tevent_req_done(req);
+               return tevent_req_post(req, ev);
        }
 
+       channel->pending_req = req;
+       channel->ev = ev;
+
        msg_tdb_event = messaging_tdb_event(state, channel->msg, ev);
        if (tevent_req_nomem(msg_tdb_event, req)) {
                return tevent_req_post(req, ev);
@@ -271,42 +268,6 @@ struct tevent_req *msg_read_send(TALLOC_CTX *mem_ctx,
        return req;
 }
 
-static int msg_read_state_destructor(struct msg_read_state *s)
-{
-       assert(s->channel->pending_req == s->req);
-       s->channel->pending_req = NULL;
-       return 0;
-}
-
-static void msg_channel_trigger(struct tevent_context *ev,
-                              struct tevent_immediate *im,
-                              void *priv)
-{
-       struct msg_channel *channel;
-       struct tevent_req *req;
-       struct msg_read_state *state;
-       size_t num_msgs;
-
-       channel = talloc_get_type_abort(priv, struct msg_channel);
-       req = channel->pending_req;
-       state = tevent_req_data(req, struct msg_read_state);
-
-       talloc_set_destructor(state, NULL);
-       msg_read_state_destructor(state);
-
-       num_msgs = talloc_array_length(channel->msgs);
-       assert(num_msgs > 0);
-
-       state->rec = talloc_move(state, &channel->msgs[0]);
-
-       memmove(channel->msgs, channel->msgs+1,
-               sizeof(struct messaging_rec *) * (num_msgs-1));
-       channel->msgs = talloc_realloc(
-               channel, channel->msgs, struct messaging_rec *, num_msgs - 1);
-
-       tevent_req_done(req);
-}
-
 static void msg_read_got_ctdb(struct tevent_req *subreq)
 {
        struct tevent_req *req = tevent_req_callback_data(
@@ -368,5 +329,6 @@ int msg_read_recv(struct tevent_req *req, TALLOC_CTX 
*mem_ctx,
                return err;
        }
        *prec = talloc_move(mem_ctx, &state->rec);
+       tevent_req_received(req);
        return 0;
 }
diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py
index baa0f2f..df88f9d 100755
--- a/source3/selftest/tests.py
+++ b/source3/selftest/tests.py
@@ -64,6 +64,7 @@ tests = ["FDPASS", "LOCK1", "LOCK2", "LOCK3", "LOCK4", 
"LOCK5", "LOCK6", "LOCK7"
         "CLEANUP1",
         "CLEANUP2",
         "CLEANUP4",
+        "LOCAL-MSG2",
         "BAD-NBT-SESSION"]
 
 for t in tests:
diff --git a/source3/torture/proto.h b/source3/torture/proto.h
index d430aef..b7eacdf 100644
--- a/source3/torture/proto.h
+++ b/source3/torture/proto.h
@@ -107,6 +107,7 @@ bool run_cleanup3(int dummy);
 bool run_cleanup4(int dummy);
 bool run_ctdb_conn(int dummy);
 bool run_msg_test(int dummy);
+bool run_msg_test2(int dummy);
 bool run_notify_bench2(int dummy);
 bool run_notify_bench3(int dummy);
 bool run_dbwrap_watch1(int dummy);
diff --git a/source3/torture/test_msg.c b/source3/torture/test_msg.c
index 2171598..d57379d 100644
--- a/source3/torture/test_msg.c
+++ b/source3/torture/test_msg.c
@@ -129,3 +129,89 @@ bool run_msg_test(int dummy)
        TALLOC_FREE(ev);
        return (ret == 0);
 }
+
+/*
+ * Reproducer for bug 10284
+ */
+
+static void msg_callback(struct tevent_req *subreq);
+
+struct msg_test2_state {
+       struct tevent_context *ev;
+       struct messaging_context *msg;
+       struct msg_channel *channel;
+       struct messaging_rec *rec;
+       struct tevent_req *req;
+};
+
+bool run_msg_test2(int dummy)
+{
+       struct msg_test2_state s;
+       NTSTATUS status;
+       int i, ret;
+
+       s.ev = samba_tevent_context_init(talloc_tos());
+       if (s.ev == NULL) {
+               fprintf(stderr, "tevent_context_init failed\n");
+               return false;
+       }
+
+       s.msg = messaging_init(s.ev, s.ev);
+       if (s.msg == NULL) {
+               fprintf(stderr, "messaging_init failed\n");
+               return false;
+       }
+
+       ret = msg_channel_init(s.ev, s.msg, MSG_PING, &s.channel);
+       if (ret != 0) {
+               fprintf(stderr, "msg_channel_init returned %s\n",
+                       strerror(ret));
+               return false;
+       }
+
+       status = messaging_send(s.msg, messaging_server_id(s.msg), MSG_PING,
+                               &data_blob_null);
+       if (!NT_STATUS_IS_OK(status)) {
+               fprintf(stderr, "messaging_send returned %s\n",
+                       nt_errstr(status));
+               return false;
+       }
+
+       ret = tevent_loop_once(s.ev);
+       if (ret == -1) {
+               fprintf(stderr, "tevent_loop_once failed: %s\n",
+                       strerror(errno));
+               return false;
+       }
+
+       s.req = msg_read_send(s.ev, s.ev, s.channel);
+       if (s.req == NULL) {
+               fprintf(stderr, "msg_read_send failed\n");
+               return false;
+       }
+       tevent_req_set_callback(s.req, msg_callback, &s);
+
+       status = messaging_send(s.msg, messaging_server_id(s.msg), MSG_PING,
+                               &data_blob_null);
+       if (!NT_STATUS_IS_OK(status)) {
+               fprintf(stderr, "messaging_send returned %s\n",
+                       nt_errstr(status));
+               return false;
+       }
+
+       for (i=0; i<5; i++) {
+               tevent_loop_once(s.ev);
+       }
+
+       return true;
+}
+
+static void msg_callback(struct tevent_req *subreq)
+{
+       struct msg_test2_state *s = _tevent_req_callback_data(subreq);
+       struct messaging_rec *rec;
+       msg_read_recv(subreq, NULL, &rec);
+       TALLOC_FREE(subreq);
+       subreq = msg_read_send(s->ev, s->ev, s->channel);
+       tevent_req_set_callback(subreq, msg_callback, s);
+}
diff --git a/source3/torture/torture.c b/source3/torture/torture.c
index 37a44b2..6789d85 100644
--- a/source3/torture/torture.c
+++ b/source3/torture/torture.c
@@ -9573,6 +9573,7 @@ static struct {
        { "LOCAL-TALLOC-DICT", run_local_talloc_dict, 0},
        { "LOCAL-CTDB-CONN", run_ctdb_conn, 0},
        { "LOCAL-MSG", run_msg_test, 0},
+       { "LOCAL-MSG2", run_msg_test2, 0},
        { "LOCAL-DBWRAP-WATCH1", run_dbwrap_watch1, 0 },
        { "LOCAL-BASE64", run_local_base64, 0},
        { "LOCAL-RBTREE", run_local_rbtree, 0},


-- 
Samba Shared Repository

Reply via email to