The branch, master has been updated
via 042f94b torture3: Add a test deleting a different req
via 54118d2 torture3: Add local-messaging-read1
via d0590ee messaging3: Fix messaging_read_send/recv
via 8117dcd messaging3: Make "presult" optional in messaging_read_recv
via 3af130e torture3: Add a bit more coverage to messaging_read
via 93796a3 messaging3: Fix formatting
via 41c5277 s3: smbd : Fix wildcard unlink to fail if we get an error
rather than trying to continue.
from 77b04f1 winbind: Allow winbindd to be run from inside "samba"
http://gitweb.samba.org/?p=samba.git;a=shortlog;h=master
- Log -----------------------------------------------------------------
commit 042f94b560344bd5481a334e5c552826c76749e2
Author: Volker Lendecke <[email protected]>
Date: Tue Apr 29 14:27:03 2014 +0200
torture3: Add a test deleting a different req
Signed-off-by: Volker Lendecke <[email protected]>
Reviewed-by: Jeremy Allison <[email protected]>
Autobuild-User(master): Volker Lendecke <[email protected]>
Autobuild-Date(master): Wed Apr 30 17:09:59 CEST 2014 on sn-devel-104
commit 54118d24a712172c6d54b213c70c926148fd61ff
Author: Volker Lendecke <[email protected]>
Date: Tue Apr 29 14:25:14 2014 +0200
torture3: Add local-messaging-read1
This covers deleting and re-adding a request in a callback
Signed-off-by: Volker Lendecke <[email protected]>
Reviewed-by: Jeremy Allison <[email protected]>
commit d0590eec7e12f2a5ddb9f76365d5547cc3c668c1
Author: Volker Lendecke <[email protected]>
Date: Tue Apr 29 14:14:24 2014 +0200
messaging3: Fix messaging_read_send/recv
messaging_read_send/recv was okay for just one handler in the queue. For
multiple handlers it was pretty broken.
A handler that deletes itself as part of the callback (pretty typical use
case...) drops the message for a subsequent handler that responds to the
same
message type. In messaging_dispatch_rec we walk the array, however
messaging_read_cleanup has already changed the array.
tevent_req_defer_callback
does not help here: It only defers the callback, it does not defer the
cleanup
function.
This also happens when a callback deletes a different handler
A handler that re-installs itself in the callback might get a message twice.
This patch changes the code such that only messaging_dispatch_rec adds
records
to msg_ctx->waiters, new waiters are put into a staging area first
(msg_ctx->new_waiters). Also messaging_read_cleanup does not move anything
around in msg_ctx->waiters, it only nulls out itself.
messaging_dispatch_rec is
changed to cope with this.
Signed-off-by: Volker Lendecke <[email protected]>
Reviewed-by: Jeremy Allison <[email protected]>
commit 8117dcd5fb9ecda8ce85c064f0ceb4fda29c8e6f
Author: Volker Lendecke <[email protected]>
Date: Tue Apr 29 14:12:26 2014 +0200
messaging3: Make "presult" optional in messaging_read_recv
Callers might not be interested in the rec, just the fact that something
arrived
Signed-off-by: Volker Lendecke <[email protected]>
Reviewed-by: Jeremy Allison <[email protected]>
commit 3af130ea931f58aa4175c7ca185eb7c539cadbd0
Author: Volker Lendecke <[email protected]>
Date: Tue Apr 29 14:10:04 2014 +0200
torture3: Add a bit more coverage to messaging_read
Signed-off-by: Volker Lendecke <[email protected]>
Reviewed-by: Jeremy Allison <[email protected]>
commit 93796a38af1c1211ac1066549899052ae4479556
Author: Volker Lendecke <[email protected]>
Date: Tue Apr 29 14:08:29 2014 +0200
messaging3: Fix formatting
This went over the 80-char limit
Signed-off-by: Volker Lendecke <[email protected]>
Reviewed-by: Jeremy Allison <[email protected]>
commit 41c5277f57eac320efb99f22b8336b13d88e8e26
Author: Jeremy Allison <[email protected]>
Date: Tue Apr 29 16:59:55 2014 -0700
s3: smbd : Fix wildcard unlink to fail if we get an error rather than
trying to continue.
This can break smbd if we end up leaving a SHARING_VIOLATION
retry record on the queue.
Signed-off-by: Jeremy Allison <[email protected]>
Reviewed-by: Volker Lendecke <[email protected]>
-----------------------------------------------------------------------
Summary of changes:
source3/include/messages.h | 3 +
source3/lib/messages.c | 97 +++++++++++---
source3/selftest/tests.py | 2 +
source3/smbd/reply.c | 3 +-
source3/torture/proto.h | 2 +
source3/torture/test_dbwrap_watch.c | 7 +
source3/torture/test_messaging_read.c | 247 +++++++++++++++++++++++++++++++++
source3/torture/torture.c | 2 +
source3/wscript_build | 1 +
9 files changed, 345 insertions(+), 19 deletions(-)
create mode 100644 source3/torture/test_messaging_read.c
Changeset truncated at 500 lines:
diff --git a/source3/include/messages.h b/source3/include/messages.h
index 1681ec9..06c1748 100644
--- a/source3/include/messages.h
+++ b/source3/include/messages.h
@@ -76,6 +76,9 @@ struct messaging_context {
struct tevent_context *event_ctx;
struct messaging_callback *callbacks;
+ struct tevent_req **new_waiters;
+ unsigned num_new_waiters;
+
struct tevent_req **waiters;
unsigned num_waiters;
diff --git a/source3/lib/messages.c b/source3/lib/messages.c
index b6fe423..9284ac1 100644
--- a/source3/lib/messages.c
+++ b/source3/lib/messages.c
@@ -475,7 +475,7 @@ struct tevent_req *messaging_read_send(TALLOC_CTX *mem_ctx,
{
struct tevent_req *req;
struct messaging_read_state *state;
- size_t waiters_len;
+ size_t new_waiters_len;
req = tevent_req_create(mem_ctx, &state,
struct messaging_read_state);
@@ -486,21 +486,21 @@ struct tevent_req *messaging_read_send(TALLOC_CTX
*mem_ctx,
state->msg_ctx = msg_ctx;
state->msg_type = msg_type;
- waiters_len = talloc_array_length(msg_ctx->waiters);
+ new_waiters_len = talloc_array_length(msg_ctx->new_waiters);
- if (waiters_len == msg_ctx->num_waiters) {
+ if (new_waiters_len == msg_ctx->num_new_waiters) {
struct tevent_req **tmp;
- tmp = talloc_realloc(msg_ctx, msg_ctx->waiters,
- struct tevent_req *, waiters_len+1);
+ tmp = talloc_realloc(msg_ctx, msg_ctx->new_waiters,
+ struct tevent_req *, new_waiters_len+1);
if (tevent_req_nomem(tmp, req)) {
return tevent_req_post(req, ev);
}
- msg_ctx->waiters = tmp;
+ msg_ctx->new_waiters = tmp;
}
- msg_ctx->waiters[msg_ctx->num_waiters] = req;
- msg_ctx->num_waiters += 1;
+ msg_ctx->new_waiters[msg_ctx->num_new_waiters] = req;
+ msg_ctx->num_new_waiters += 1;
tevent_req_set_cleanup_fn(req, messaging_read_cleanup);
return req;
@@ -512,21 +512,27 @@ static void messaging_read_cleanup(struct tevent_req *req,
struct messaging_read_state *state = tevent_req_data(
req, struct messaging_read_state);
struct messaging_context *msg_ctx = state->msg_ctx;
- struct tevent_req **waiters = msg_ctx->waiters;
unsigned i;
tevent_req_set_cleanup_fn(req, NULL);
for (i=0; i<msg_ctx->num_waiters; i++) {
- if (waiters[i] == req) {
- waiters[i] = waiters[msg_ctx->num_waiters-1];
- msg_ctx->num_waiters -= 1;
+ if (msg_ctx->waiters[i] == req) {
+ msg_ctx->waiters[i] = NULL;
+ return;
+ }
+ }
+
+ for (i=0; i<msg_ctx->num_new_waiters; i++) {
+ if (msg_ctx->new_waiters[i] == req) {
+ msg_ctx->new_waiters[i] = NULL;
return;
}
}
}
-static void messaging_read_done(struct tevent_req *req, struct messaging_rec
*rec)
+static void messaging_read_done(struct tevent_req *req,
+ struct messaging_rec *rec)
{
struct messaging_read_state *state = tevent_req_data(
req, struct messaging_read_state);
@@ -549,10 +555,40 @@ int messaging_read_recv(struct tevent_req *req,
TALLOC_CTX *mem_ctx,
tevent_req_received(req);
return err;
}
- *presult = talloc_move(mem_ctx, &state->rec);
+ if (presult != NULL) {
+ *presult = talloc_move(mem_ctx, &state->rec);
+ }
return 0;
}
+static bool messaging_append_new_waiters(struct messaging_context *msg_ctx)
+{
+ if (msg_ctx->num_new_waiters == 0) {
+ return true;
+ }
+
+ if (talloc_array_length(msg_ctx->waiters) <
+ (msg_ctx->num_waiters + msg_ctx->num_new_waiters)) {
+ struct tevent_req **tmp;
+ tmp = talloc_realloc(
+ msg_ctx, msg_ctx->waiters, struct tevent_req *,
+ msg_ctx->num_waiters + msg_ctx->num_new_waiters);
+ if (tmp == NULL) {
+ DEBUG(1, ("%s: talloc failed\n", __func__));
+ return false;
+ }
+ msg_ctx->waiters = tmp;
+ }
+
+ memcpy(&msg_ctx->waiters[msg_ctx->num_waiters], msg_ctx->new_waiters,
+ sizeof(struct tevent_req *) * msg_ctx->num_new_waiters);
+
+ msg_ctx->num_waiters += msg_ctx->num_new_waiters;
+ msg_ctx->num_new_waiters = 0;
+
+ return true;
+}
+
/*
Dispatch one messaging_rec
*/
@@ -575,14 +611,39 @@ void messaging_dispatch_rec(struct messaging_context
*msg_ctx,
}
}
- for (i=0; i<msg_ctx->num_waiters; i++) {
- struct tevent_req *req = msg_ctx->waiters[i];
- struct messaging_read_state *state = tevent_req_data(
- req, struct messaging_read_state);
+ if (!messaging_append_new_waiters(msg_ctx)) {
+ return;
+ }
+
+ i = 0;
+ while (i < msg_ctx->num_waiters) {
+ struct tevent_req *req;
+ struct messaging_read_state *state;
+
+ req = msg_ctx->waiters[i];
+ if (req == NULL) {
+ /*
+ * This got cleaned up. In the meantime,
+ * move everything down one. We need
+ * to keep the order of waiters, as
+ * other code may depend on this.
+ */
+ if (i < msg_ctx->num_waiters - 1) {
+ memmove(&msg_ctx->waiters[i],
+ &msg_ctx->waiters[i+1],
+ sizeof(struct tevent_req *) *
+ (msg_ctx->num_waiters - i - 1));
+ }
+ msg_ctx->num_waiters -= 1;
+ continue;
+ }
+ state = tevent_req_data(req, struct messaging_read_state);
if (state->msg_type == rec->msg_type) {
messaging_read_done(req, rec);
}
+
+ i += 1;
}
return;
}
diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py
index 4357ed9..fcf3cd5 100755
--- a/source3/selftest/tests.py
+++ b/source3/selftest/tests.py
@@ -102,6 +102,8 @@ local_tests = [
"LOCAL-CONVERT-STRING",
"LOCAL-CONV-AUTH-INFO",
"LOCAL-IDMAP-TDB-COMMON",
+ "LOCAL-MESSAGING-READ1",
+ "LOCAL-MESSAGING-READ2",
"LOCAL-hex_encode_buf",
"LOCAL-sprintf_append",
"LOCAL-remove_duplicate_addrs2"]
diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c
index da59ca7..f737d74 100644
--- a/source3/smbd/reply.c
+++ b/source3/smbd/reply.c
@@ -2909,9 +2909,10 @@ NTSTATUS unlink_internals(connection_struct *conn,
struct smb_request *req,
status = do_unlink(conn, req, smb_fname, dirtype);
if (!NT_STATUS_IS_OK(status)) {
+ TALLOC_FREE(dir_hnd);
TALLOC_FREE(frame);
TALLOC_FREE(talloced);
- continue;
+ goto out;
}
count++;
diff --git a/source3/torture/proto.h b/source3/torture/proto.h
index 3673d98..a737ea4 100644
--- a/source3/torture/proto.h
+++ b/source3/torture/proto.h
@@ -113,5 +113,7 @@ bool run_idmap_tdb_common_test(int dummy);
bool run_local_dbwrap_ctdb(int dummy);
bool run_qpathinfo_bufsize(int dummy);
bool run_bench_pthreadpool(int dummy);
+bool run_messaging_read1(int dummy);
+bool run_messaging_read2(int dummy);
#endif /* __TORTURE_H__ */
diff --git a/source3/torture/test_dbwrap_watch.c
b/source3/torture/test_dbwrap_watch.c
index 4e699fe..ab9330f 100644
--- a/source3/torture/test_dbwrap_watch.c
+++ b/source3/torture/test_dbwrap_watch.c
@@ -67,6 +67,13 @@ bool run_dbwrap_watch1(int dummy)
}
TALLOC_FREE(rec);
+ status = dbwrap_store_int32_bystring(db, "different_key", 1);
+ if (!NT_STATUS_IS_OK(status)) {
+ fprintf(stderr, "dbwrap_store_int32 failed: %s\n",
+ nt_errstr(status));
+ goto fail;
+ }
+
status = dbwrap_store_int32_bystring(db, keystr, 1);
if (!NT_STATUS_IS_OK(status)) {
fprintf(stderr, "dbwrap_store_int32 failed: %s\n",
diff --git a/source3/torture/test_messaging_read.c
b/source3/torture/test_messaging_read.c
new file mode 100644
index 0000000..387ebfd
--- /dev/null
+++ b/source3/torture/test_messaging_read.c
@@ -0,0 +1,247 @@
+/*
+ Unix SMB/CIFS implementation.
+ Test for a messaging_read bug
+ Copyright (C) Volker Lendecke 2014
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>.
+*/
+
+#include "includes.h"
+#include "torture/proto.h"
+#include "lib/util/tevent_unix.h"
+#include "messages.h"
+
+struct msg_count_state {
+ struct tevent_context *ev;
+ struct messaging_context *msg_ctx;
+ uint32_t msg_type;
+ unsigned *count;
+};
+
+static void msg_count_done(struct tevent_req *subreq);
+
+static struct tevent_req *msg_count_send(TALLOC_CTX *mem_ctx,
+ struct tevent_context *ev,
+ struct messaging_context *msg_ctx,
+ uint32_t msg_type,
+ unsigned *count)
+{
+ struct tevent_req *req, *subreq;
+ struct msg_count_state *state;
+
+ req = tevent_req_create(mem_ctx, &state, struct msg_count_state);
+ if (req == NULL) {
+ return NULL;
+ }
+ state->ev = ev;
+ state->msg_ctx = msg_ctx;
+ state->msg_type = msg_type;
+ state->count = count;
+
+ subreq = messaging_read_send(state, state->ev, state->msg_ctx,
+ state->msg_type);
+ if (tevent_req_nomem(subreq, req)) {
+ return tevent_req_post(req, ev);
+ }
+ tevent_req_set_callback(subreq, msg_count_done, req);
+ return req;
+}
+
+static void msg_count_done(struct tevent_req *subreq)
+{
+ struct tevent_req *req = tevent_req_callback_data(
+ subreq, struct tevent_req);
+ struct msg_count_state *state = tevent_req_data(
+ req, struct msg_count_state);
+ int ret;
+
+ ret = messaging_read_recv(subreq, NULL, NULL);
+ TALLOC_FREE(subreq);
+ if (tevent_req_error(req, ret)) {
+ return;
+ }
+ *state->count += 1;
+
+ subreq = messaging_read_send(state, state->ev, state->msg_ctx,
+ state->msg_type);
+ if (tevent_req_nomem(subreq, req)) {
+ return;
+ }
+ tevent_req_set_callback(subreq, msg_count_done, req);
+}
+
+bool run_messaging_read1(int dummy)
+{
+ struct tevent_context *ev = NULL;
+ struct messaging_context *msg_ctx = NULL;
+ struct tevent_req *req1 = NULL;
+ unsigned count1 = 0;
+ struct tevent_req *req2 = NULL;
+ unsigned count2 = 0;
+ NTSTATUS status;
+ bool retval = false;
+
+ ev = samba_tevent_context_init(talloc_tos());
+ if (ev == NULL) {
+ fprintf(stderr, "tevent_context_init failed\n");
+ goto fail;
+ }
+ msg_ctx = messaging_init(ev, ev);
+ if (msg_ctx == NULL) {
+ fprintf(stderr, "messaging_init failed\n");
+ goto fail;
+ }
+
+ req1 = msg_count_send(ev, ev, msg_ctx, MSG_SMB_NOTIFY, &count1);
+ if (req1 == NULL) {
+ fprintf(stderr, "msg_count_send failed\n");
+ goto fail;
+ }
+ req2 = msg_count_send(ev, ev, msg_ctx, MSG_SMB_NOTIFY, &count2);
+ if (req1 == NULL) {
+ fprintf(stderr, "msg_count_send failed\n");
+ goto fail;
+ }
+ status = messaging_send_buf(msg_ctx, messaging_server_id(msg_ctx),
+ MSG_SMB_NOTIFY, NULL, 0);
+ if (!NT_STATUS_IS_OK(status)) {
+ fprintf(stderr, "messaging_send_buf failed: %s\n",
+ nt_errstr(status));
+ goto fail;
+ }
+
+ if (tevent_loop_once(ev) != 0) {
+ fprintf(stderr, "tevent_loop_once failed\n");
+ goto fail;
+ }
+
+ printf("%u/%u\n", count1, count2);
+
+ if ((count1 != 1) || (count2 != 1)){
+ fprintf(stderr, "Got %u/%u msgs, expected 1 each\n",
+ count1, count2);
+ goto fail;
+ }
+
+ retval = true;
+fail:
+ TALLOC_FREE(req1);
+ TALLOC_FREE(req2);
+ TALLOC_FREE(msg_ctx);
+ TALLOC_FREE(ev);
+ return retval;
+}
+
+struct msg_free_state {
+ struct tevent_req **to_free;
+};
+
+static void msg_free_done(struct tevent_req *subreq);
+
+static struct tevent_req *msg_free_send(TALLOC_CTX *mem_ctx,
+ struct tevent_context *ev,
+ struct messaging_context *msg_ctx,
+ uint32_t msg_type,
+ struct tevent_req **to_free)
+{
+ struct tevent_req *req, *subreq;
+ struct msg_free_state *state;
+
+ req = tevent_req_create(mem_ctx, &state, struct msg_free_state);
+ if (req == NULL) {
+ return NULL;
+ }
+ state->to_free = to_free;
+
+ subreq = messaging_read_send(state, ev, msg_ctx, msg_type);
+ if (tevent_req_nomem(subreq, req)) {
+ return tevent_req_post(req, ev);
+ }
+ tevent_req_set_callback(subreq, msg_free_done, req);
+ return req;
+}
+
+static void msg_free_done(struct tevent_req *subreq)
+{
+ struct tevent_req *req = tevent_req_callback_data(
+ subreq, struct tevent_req);
+ struct msg_free_state *state = tevent_req_data(
+ req, struct msg_free_state);
+ int ret;
+
+ ret = messaging_read_recv(subreq, NULL, NULL);
+ TALLOC_FREE(subreq);
+ if (tevent_req_error(req, ret)) {
+ return;
+ }
+ TALLOC_FREE(*state->to_free);
+ tevent_req_done(req);
+}
+
+bool run_messaging_read2(int dummy)
+{
+ struct tevent_context *ev = NULL;
+ struct messaging_context *msg_ctx = NULL;
+ struct tevent_req *req1 = NULL;
+ struct tevent_req *req2 = NULL;
+ unsigned count = 0;
+ NTSTATUS status;
+ bool retval = false;
+
+ ev = samba_tevent_context_init(talloc_tos());
+ if (ev == NULL) {
+ fprintf(stderr, "tevent_context_init failed\n");
+ goto fail;
+ }
+ msg_ctx = messaging_init(ev, ev);
+ if (msg_ctx == NULL) {
+ fprintf(stderr, "messaging_init failed\n");
+ goto fail;
+ }
+
+ req1 = msg_free_send(ev, ev, msg_ctx, MSG_SMB_NOTIFY, &req2);
+ if (req1 == NULL) {
+ fprintf(stderr, "msg_count_send failed\n");
+ goto fail;
+ }
+ req2 = msg_count_send(ev, ev, msg_ctx, MSG_SMB_NOTIFY, &count);
+ if (req1 == NULL) {
+ fprintf(stderr, "msg_count_send failed\n");
+ goto fail;
+ }
+ status = messaging_send_buf(msg_ctx, messaging_server_id(msg_ctx),
+ MSG_SMB_NOTIFY, NULL, 0);
+ if (!NT_STATUS_IS_OK(status)) {
+ fprintf(stderr, "messaging_send_buf failed: %s\n",
+ nt_errstr(status));
+ goto fail;
+ }
+
+ if (!tevent_req_poll(req1, ev) != 0) {
+ fprintf(stderr, "tevent_req_poll failed\n");
+ goto fail;
+ }
+
+ if (count != 0) {
+ fprintf(stderr, "Got %u msgs, expected none\n", count);
+ goto fail;
+ }
+
+ retval = true;
+fail:
+ TALLOC_FREE(req1);
+ TALLOC_FREE(msg_ctx);
+ TALLOC_FREE(ev);
+ return retval;
+}
diff --git a/source3/torture/torture.c b/source3/torture/torture.c
index 61e9338..f97119a 100644
--- a/source3/torture/torture.c
+++ b/source3/torture/torture.c
@@ -9574,6 +9574,8 @@ static struct {
{ "LOCAL-TALLOC-DICT", run_local_talloc_dict, 0},
{ "LOCAL-CTDB-CONN", run_ctdb_conn, 0},
{ "LOCAL-DBWRAP-WATCH1", run_dbwrap_watch1, 0 },
+ { "LOCAL-MESSAGING-READ1", run_messaging_read1, 0 },
+ { "LOCAL-MESSAGING-READ2", run_messaging_read2, 0 },
{ "LOCAL-BASE64", run_local_base64, 0},
{ "LOCAL-RBTREE", run_local_rbtree, 0},
{ "LOCAL-MEMCACHE", run_local_memcache, 0},
--
Samba Shared Repository
