From 48ec03952ab0663257a846838272ba213683ee6a Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Wed, 8 Mar 2017 14:53:26 +1300 Subject: [PATCH 1/7] pymessaging: Add support for irpc_add_name Signed-off-by: Andrew Bartlett Pair-Programmed-by: Gary Lockyer Signed-off-by: Gary Lockyer --- python/samba/tests/messaging.py | 4 ++++ source4/lib/messaging/pymessaging.c | 23 ++++++++++++++++++++++- 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/python/samba/tests/messaging.py b/python/samba/tests/messaging.py index 5d32d60..1c5dfe5 100644 --- a/python/samba/tests/messaging.py +++ b/python/samba/tests/messaging.py @@ -49,6 +49,10 @@ class MessagingTests(TestCase): x = self.get_context() self.assertTrue(isinstance(x.server_id, server_id)) + def test_add_name(self): + x = self.get_context() + x.irpc_add_name("samba.messaging test") + def test_ping_speed(self): server_ctx = self.get_context((0, 1)) def ping_callback(src, data): diff --git a/source4/lib/messaging/pymessaging.c b/source4/lib/messaging/pymessaging.c index f62354b..9d0997f 100644 --- a/source4/lib/messaging/pymessaging.c +++ b/source4/lib/messaging/pymessaging.c @@ -241,6 +241,25 @@ static PyObject *py_imessaging_deregister(PyObject *self, PyObject *args, PyObje Py_RETURN_NONE; } +static PyObject *py_irpc_add_name(PyObject *self, PyObject *args, PyObject *kwargs) +{ + imessaging_Object *iface = (imessaging_Object *)self; + char *server_name; + NTSTATUS status; + + if (!PyArg_ParseTuple(args, "s", &server_name)) { + return NULL; + } + + status = irpc_add_name(iface->msg_ctx, server_name); + if (!NT_STATUS_IS_OK(status)) { + PyErr_SetNTSTATUS(status); + return NULL; + } + + Py_RETURN_NONE; +} + static PyObject *py_irpc_servers_byname(PyObject *self, PyObject *args, PyObject *kwargs) { imessaging_Object *iface = (imessaging_Object *)self; @@ -341,10 +360,12 @@ static PyMethodDef py_imessaging_methods[] = { "S.register(callback, msg_type=None) -> msg_type\nRegister a message handler" }, { "deregister", (PyCFunction)py_imessaging_deregister, METH_VARARGS|METH_KEYWORDS, "S.deregister(callback, msg_type) -> None\nDeregister a message handler" }, + { "irpc_add_name", (PyCFunction)py_irpc_add_name, METH_VARARGS, + "S.irpc_add_name(name) -> None\nAdd this context to the list of server_id values that are registered for a particular name" }, { "irpc_servers_byname", (PyCFunction)py_irpc_servers_byname, METH_VARARGS, "S.irpc_servers_byname(name) -> list\nGet list of server_id values that are registered for a particular name" }, { "irpc_all_servers", (PyCFunction)py_irpc_all_servers, METH_NOARGS, - "S.irpc_servers_byname() -> list\nGet list of all registered names and the associated server_id values" }, + "S.irpc_all_servers() -> list\nGet list of all registered names and the associated server_id values" }, { NULL, NULL, 0, NULL } }; -- 2.9.3 From 461fc025fba11d3ff6b723b8aa421c7d94083ce7 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Tue, 14 Mar 2017 13:39:00 +1300 Subject: [PATCH 2/7] pymessaging: Add irpc_remove_name Signed-off-by: Andrew Bartlett --- python/samba/tests/messaging.py | 8 ++++++++ source4/lib/messaging/pymessaging.c | 16 ++++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/python/samba/tests/messaging.py b/python/samba/tests/messaging.py index 1c5dfe5..3eeab52 100644 --- a/python/samba/tests/messaging.py +++ b/python/samba/tests/messaging.py @@ -22,6 +22,7 @@ import samba from samba.messaging import Messaging from samba.tests import TestCase from samba.dcerpc.server_id import server_id +from samba.ndr import ndr_print class MessagingTests(TestCase): @@ -52,6 +53,13 @@ class MessagingTests(TestCase): def test_add_name(self): x = self.get_context() x.irpc_add_name("samba.messaging test") + name_list = x.irpc_servers_byname("samba.messaging test") + self.assertEqual(len(name_list), 1) + self.assertEqual(ndr_print(x.server_id), + ndr_print(name_list[0])) + x.irpc_remove_name("samba.messaging test") + self.assertEqual([], + x.irpc_servers_byname("samba.messaging test")) def test_ping_speed(self): server_ctx = self.get_context((0, 1)) diff --git a/source4/lib/messaging/pymessaging.c b/source4/lib/messaging/pymessaging.c index 9d0997f..b317955 100644 --- a/source4/lib/messaging/pymessaging.c +++ b/source4/lib/messaging/pymessaging.c @@ -260,6 +260,20 @@ static PyObject *py_irpc_add_name(PyObject *self, PyObject *args, PyObject *kwar Py_RETURN_NONE; } +static PyObject *py_irpc_remove_name(PyObject *self, PyObject *args, PyObject *kwargs) +{ + imessaging_Object *iface = (imessaging_Object *)self; + char *server_name; + + if (!PyArg_ParseTuple(args, "s", &server_name)) { + return NULL; + } + + irpc_remove_name(iface->msg_ctx, server_name); + + Py_RETURN_NONE; +} + static PyObject *py_irpc_servers_byname(PyObject *self, PyObject *args, PyObject *kwargs) { imessaging_Object *iface = (imessaging_Object *)self; @@ -362,6 +376,8 @@ static PyMethodDef py_imessaging_methods[] = { "S.deregister(callback, msg_type) -> None\nDeregister a message handler" }, { "irpc_add_name", (PyCFunction)py_irpc_add_name, METH_VARARGS, "S.irpc_add_name(name) -> None\nAdd this context to the list of server_id values that are registered for a particular name" }, + { "irpc_remove_name", (PyCFunction)py_irpc_remove_name, METH_VARARGS, + "S.irpc_remove_name(name) -> None\nAdd this context to the list of server_id values that are registered for a particular name" }, { "irpc_servers_byname", (PyCFunction)py_irpc_servers_byname, METH_VARARGS, "S.irpc_servers_byname(name) -> list\nGet list of server_id values that are registered for a particular name" }, { "irpc_all_servers", (PyCFunction)py_irpc_all_servers, METH_NOARGS, -- 2.9.3 From c540d344b987f688c8fbdcbef87c85b0c8cd525f Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Tue, 14 Mar 2017 16:07:46 +1300 Subject: [PATCH 3/7] selftest: Test server_id database add and removal Signed-off-by: Andrew Bartlett --- python/samba/tests/messaging.py | 19 +++++++++++++------ selftest/knownfail | 1 + 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/python/samba/tests/messaging.py b/python/samba/tests/messaging.py index 3eeab52..a70be96 100644 --- a/python/samba/tests/messaging.py +++ b/python/samba/tests/messaging.py @@ -23,6 +23,7 @@ from samba.messaging import Messaging from samba.tests import TestCase from samba.dcerpc.server_id import server_id from samba.ndr import ndr_print +import random class MessagingTests(TestCase): @@ -46,20 +47,26 @@ class MessagingTests(TestCase): for name in x.irpc_all_servers(): self.assertTrue(isinstance(x.irpc_servers_byname(name.name), list)) + def test_unknown_name(self): + x = self.get_context() + self.assertRaises(KeyError, + x.irpc_servers_byname, "samba.messaging test NONEXISTING") + def test_assign_server_id(self): x = self.get_context() self.assertTrue(isinstance(x.server_id, server_id)) - def test_add_name(self): + def test_add_remove_name(self): x = self.get_context() - x.irpc_add_name("samba.messaging test") - name_list = x.irpc_servers_byname("samba.messaging test") + name = "samba.messaging test-%d" % random.randint(1, 1000000) + x.irpc_add_name(name) + name_list = x.irpc_servers_byname(name) self.assertEqual(len(name_list), 1) self.assertEqual(ndr_print(x.server_id), ndr_print(name_list[0])) - x.irpc_remove_name("samba.messaging test") - self.assertEqual([], - x.irpc_servers_byname("samba.messaging test")) + x.irpc_remove_name(name) + self.assertRaises(KeyError, + x.irpc_servers_byname, name) def test_ping_speed(self): server_ctx = self.get_context((0, 1)) diff --git a/selftest/knownfail b/selftest/knownfail index cfd4b35..2f3b22b 100644 --- a/selftest/knownfail +++ b/selftest/knownfail @@ -317,3 +317,4 @@ ^samba3.smb2.credits.skipped_mid.* ^samba4.blackbox.dbcheck-links.release-4-5-0-pre1.dangling_multi_valued_dbcheck ^samba4.blackbox.dbcheck-links.release-4-5-0-pre1.dangling_multi_valued_check_missing +^samba.tests.messaging.samba.tests.messaging.MessagingTests.test_add_remove_name \ No newline at end of file -- 2.9.3 From eef04949817bab65aa3c3268cf8690e7753e174e Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Tue, 14 Mar 2017 15:22:01 +1300 Subject: [PATCH 4/7] lib/util: Do not return an unterminated pointer in tdb_fetch_talloc() Otherwise, if a TDB entry is truncated to 0 length, this will return uninitialised memory! Signed-off-by: Andrew Bartlett --- lib/util/server_id_db.c | 42 +++++++++++++++++++++++++++++++++--------- lib/util/util_tdb.c | 9 +++++---- lib/util/util_tdb.h | 2 +- selftest/knownfail | 1 - 4 files changed, 39 insertions(+), 15 deletions(-) diff --git a/lib/util/server_id_db.c b/lib/util/server_id_db.c index e0b8476..937de89 100644 --- a/lib/util/server_id_db.c +++ b/lib/util/server_id_db.c @@ -137,7 +137,8 @@ int server_id_db_prune_name(struct server_id_db *db, const char *name, size_t idbuf_len = server_id_str_buf_unique(server, NULL, 0); char idbuf[idbuf_len]; TDB_DATA key; - uint8_t *data; + TDB_DATA data; + TDB_DATA data_store; char *ids, *id; int ret; @@ -156,18 +157,32 @@ int server_id_db_prune_name(struct server_id_db *db, const char *name, return ret; } - ids = (char *)data; + if (data.dsize == 0) { + tdb_chainunlock(tdb, key); + TALLOC_FREE(data.dptr); + return ENOENT; + } + + /* We assert that the DB contains a NULL-terminated string */ + ids = (char *)data.dptr; id = strv_find(ids, idbuf); if (id == NULL) { tdb_chainunlock(tdb, key); - TALLOC_FREE(data); + TALLOC_FREE(data.dptr); return ENOENT; } strv_delete(&ids, id); - ret = tdb_store(tdb, key, talloc_tdb_data(ids), TDB_MODIFY); - TALLOC_FREE(data); + + data_store = talloc_tdb_data(ids); + + if (data_store.dsize == 0) { + ret = tdb_delete(tdb, key); + } else { + ret = tdb_store(tdb, key, data_store, TDB_MODIFY); + } + TALLOC_FREE(data.dptr); tdb_chainunlock(tdb, key); @@ -199,7 +214,7 @@ int server_id_db_lookup(struct server_id_db *db, const char *name, { struct tdb_context *tdb = db->tdb->tdb; TDB_DATA key; - uint8_t *data; + TDB_DATA data; char *ids, *id; unsigned num_servers; struct server_id *servers; @@ -212,12 +227,17 @@ int server_id_db_lookup(struct server_id_db *db, const char *name, return ret; } - ids = (char *)data; + if (data.dsize == 0) { + return ENOENT; + } + + /* We assert that the DB contains a NULL-terminated string */ + ids = (char *)data.dptr; num_servers = strv_count(ids); servers = talloc_array(mem_ctx, struct server_id, num_servers); if (servers == NULL) { - TALLOC_FREE(data); + TALLOC_FREE(data.dptr); return ENOMEM; } @@ -227,7 +247,7 @@ int server_id_db_lookup(struct server_id_db *db, const char *name, servers[i++] = server_id_from_string(NONCLUSTER_VNN, id); } - TALLOC_FREE(data); + TALLOC_FREE(data.dptr); *pnum_servers = num_servers; *pservers = servers; @@ -283,6 +303,10 @@ static int server_id_db_traverse_fn(struct tdb_context *tdb, } name = (const char *)key.dptr; + if (data.dsize == 0) { + return 0; + } + ids = (char *)talloc_memdup(state->mem_ctx, data.dptr, data.dsize); if (ids == NULL) { return 0; diff --git a/lib/util/util_tdb.c b/lib/util/util_tdb.c index 9bf18dc..8ff89ff 100644 --- a/lib/util/util_tdb.c +++ b/lib/util/util_tdb.c @@ -486,19 +486,20 @@ int map_unix_error_from_tdb(enum TDB_ERROR err) struct tdb_fetch_talloc_state { TALLOC_CTX *mem_ctx; - uint8_t *buf; + TDB_DATA buf; }; static int tdb_fetch_talloc_parser(TDB_DATA key, TDB_DATA data, void *private_data) { struct tdb_fetch_talloc_state *state = private_data; - state->buf = talloc_memdup(state->mem_ctx, data.dptr, data.dsize); + state->buf.dptr = talloc_memdup(state->mem_ctx, data.dptr, data.dsize); + state->buf.dsize = data.dsize; return 0; } int tdb_fetch_talloc(struct tdb_context *tdb, TDB_DATA key, - TALLOC_CTX *mem_ctx, uint8_t **buf) + TALLOC_CTX *mem_ctx, TDB_DATA *buf) { struct tdb_fetch_talloc_state state = { .mem_ctx = mem_ctx }; int ret; @@ -509,7 +510,7 @@ int tdb_fetch_talloc(struct tdb_context *tdb, TDB_DATA key, return map_unix_error_from_tdb(err); } - if (state.buf == NULL) { + if (state.buf.dptr == NULL && state.buf.dsize != 0) { return ENOMEM; } diff --git a/lib/util/util_tdb.h b/lib/util/util_tdb.h index 3b50789..1f56341 100644 --- a/lib/util/util_tdb.h +++ b/lib/util/util_tdb.h @@ -146,6 +146,6 @@ NTSTATUS map_nt_error_from_tdb(enum TDB_ERROR err); int map_unix_error_from_tdb(enum TDB_ERROR err); int tdb_fetch_talloc(struct tdb_context *tdb, TDB_DATA key, - TALLOC_CTX *mem_ctx, uint8_t **buf); + TALLOC_CTX *mem_ctx, TDB_DATA *buf); #endif /* _____LIB_UTIL_UTIL_TDB_H__ */ diff --git a/selftest/knownfail b/selftest/knownfail index 2f3b22b..cfd4b35 100644 --- a/selftest/knownfail +++ b/selftest/knownfail @@ -317,4 +317,3 @@ ^samba3.smb2.credits.skipped_mid.* ^samba4.blackbox.dbcheck-links.release-4-5-0-pre1.dangling_multi_valued_dbcheck ^samba4.blackbox.dbcheck-links.release-4-5-0-pre1.dangling_multi_valued_check_missing -^samba.tests.messaging.samba.tests.messaging.MessagingTests.test_add_remove_name \ No newline at end of file -- 2.9.3 From fd22a3fdbd6e3561f76dbad05730c78f7500a135 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Mon, 20 Mar 2017 14:57:41 +1300 Subject: [PATCH 5/7] server_id: Add runtime check for null termination Signed-off-by: Andrew Bartlett --- lib/util/server_id_db.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/lib/util/server_id_db.c b/lib/util/server_id_db.c index 937de89..8aeda98 100644 --- a/lib/util/server_id_db.c +++ b/lib/util/server_id_db.c @@ -164,6 +164,10 @@ int server_id_db_prune_name(struct server_id_db *db, const char *name, } /* We assert that the DB contains a NULL-terminated string */ + if (data.dptr[data.dsize - 1] != '\0') { + return EINVAL; + } + ids = (char *)data.dptr; id = strv_find(ids, idbuf); @@ -232,6 +236,10 @@ int server_id_db_lookup(struct server_id_db *db, const char *name, } /* We assert that the DB contains a NULL-terminated string */ + if (data.dptr[data.dsize - 1] != '\0') { + return EINVAL; + } + ids = (char *)data.dptr; num_servers = strv_count(ids); -- 2.9.3 From a78ec79a012c4b0e8face1efa4b043180b26e001 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Tue, 14 Mar 2017 12:39:13 +1300 Subject: [PATCH 6/7] pymessaging: Add a hook to run the event loop, make callbacks practical These change allow us to write a messaging server in python. The previous ping_speed test did not actually test anything, so we use .loop_once() to make it actually work. To enable practial use a context is supplied in the tuple with the callback, and the server_id for the reply is not placed inside an additional tuple. In order to get at the internal event context on which to loop, we expose imessaging_context in messaging_internal.h and allow the python bindings to use that header. Signed-off-by: Andrew Bartlett --- python/samba/tests/messaging.py | 52 +++++++++++++------ source4/lib/messaging/messaging.c | 17 +------ source4/lib/messaging/messaging_internal.h | 36 ++++++++++++++ source4/lib/messaging/pymessaging.c | 80 +++++++++++++++++++++++++++--- 4 files changed, 147 insertions(+), 38 deletions(-) create mode 100644 source4/lib/messaging/messaging_internal.h diff --git a/python/samba/tests/messaging.py b/python/samba/tests/messaging.py index a70be96..6ee18e7 100644 --- a/python/samba/tests/messaging.py +++ b/python/samba/tests/messaging.py @@ -21,8 +21,9 @@ import samba from samba.messaging import Messaging from samba.tests import TestCase -from samba.dcerpc.server_id import server_id +import time from samba.ndr import ndr_print +from samba.dcerpc import server_id import random class MessagingTests(TestCase): @@ -35,7 +36,8 @@ class MessagingTests(TestCase): x = self.get_context() def callback(): pass - msg_type = x.register(callback) + msg_type = x.register((callback, None)) + self.assertTrue(isinstance(msg_type, long)) x.deregister(callback, msg_type) def test_all_servers(self): @@ -54,7 +56,7 @@ class MessagingTests(TestCase): def test_assign_server_id(self): x = self.get_context() - self.assertTrue(isinstance(x.server_id, server_id)) + self.assertTrue(isinstance(x.server_id, server_id.server_id)) def test_add_remove_name(self): x = self.get_context() @@ -69,19 +71,41 @@ class MessagingTests(TestCase): x.irpc_servers_byname, name) def test_ping_speed(self): + got_ping = {"count": 0} + got_pong = {"count": 0} + timeout = False + + msg_pong = 0 + msg_ping = 0 + server_ctx = self.get_context((0, 1)) - def ping_callback(src, data): - server_ctx.send(src, data) - def exit_callback(): - print "received exit" - msg_ping = server_ctx.register(ping_callback) - msg_exit = server_ctx.register(exit_callback) - - def pong_callback(): - print "received pong" + def ping_callback(got_ping, msg_type, src, data): + got_ping["count"] += 1 + server_ctx.send(src, msg_pong, data) + + msg_ping = server_ctx.register((ping_callback, got_ping)) + + def pong_callback(got_pong, msg_type, src, data): + got_pong["count"] += 1 + client_ctx = self.get_context((0, 2)) - msg_pong = client_ctx.register(pong_callback) + msg_pong = client_ctx.register((pong_callback, got_pong)) + # Try both server_id forms (structure and tuple) client_ctx.send((0, 1), msg_ping, "testing") - client_ctx.send((0, 1), msg_ping, "") + client_ctx.send((0, 1), msg_ping, "testing2") + + start_time = time.time() + + # NOTE WELL: If debugging this with GDB, then the timeout will + # fire while you are trying to understand it. + + while (got_ping["count"] < 2 or got_pong["count"] < 2) and not timeout: + client_ctx.loop_once(0.1) + server_ctx.loop_once(0.1) + if time.time() - start_time > 1: + timeout = True + + self.assertEqual(got_ping["count"], 2) + self.assertEqual(got_pong["count"], 2) diff --git a/source4/lib/messaging/messaging.c b/source4/lib/messaging/messaging.c index 84df934..4d75f09 100644 --- a/source4/lib/messaging/messaging.c +++ b/source4/lib/messaging/messaging.c @@ -24,6 +24,7 @@ #include "lib/util/server_id.h" #include "system/filesys.h" #include "messaging/messaging.h" +#include "messaging/messaging_internal.h" #include "../lib/util/dlinklist.h" #include "lib/socket/socket.h" #include "librpc/gen_ndr/ndr_irpc.h" @@ -55,22 +56,6 @@ struct irpc_request { } incoming; }; -struct imessaging_context { - struct imessaging_context *prev, *next; - struct tevent_context *ev; - struct server_id server_id; - const char *sock_dir; - const char *lock_dir; - struct dispatch_fn **dispatch; - uint32_t num_types; - struct idr_context *dispatch_tree; - struct irpc_list *irpc; - struct idr_context *idr; - struct server_id_db *names; - struct timeval start_time; - void *msg_dgm_ref; -}; - /* we have a linked list of dispatch handlers for each msg_type that this messaging server can deal with */ struct dispatch_fn { diff --git a/source4/lib/messaging/messaging_internal.h b/source4/lib/messaging/messaging_internal.h new file mode 100644 index 0000000..93c5c4b --- /dev/null +++ b/source4/lib/messaging/messaging_internal.h @@ -0,0 +1,36 @@ +/* + Unix SMB/CIFS implementation. + + Samba internal messaging functions + + Copyright (C) Andrew Tridgell 2004 + + 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 . +*/ + +struct imessaging_context { + struct imessaging_context *prev, *next; + struct tevent_context *ev; + struct server_id server_id; + const char *sock_dir; + const char *lock_dir; + struct dispatch_fn **dispatch; + uint32_t num_types; + struct idr_context *dispatch_tree; + struct irpc_list *irpc; + struct idr_context *idr; + struct server_id_db *names; + struct timeval start_time; + void *msg_dgm_ref; +}; diff --git a/source4/lib/messaging/pymessaging.c b/source4/lib/messaging/pymessaging.c index b317955..84ceff5 100644 --- a/source4/lib/messaging/pymessaging.c +++ b/source4/lib/messaging/pymessaging.c @@ -34,6 +34,7 @@ #include "librpc/rpc/dcerpc.h" #include "librpc/gen_ndr/server_id.h" #include +#include "messaging_internal.h" void initmessaging(void); @@ -173,7 +174,8 @@ static void py_msg_callback_wrapper(struct imessaging_context *msg, void *privat uint32_t msg_type, struct server_id server_id, DATA_BLOB *data) { - PyObject *py_server_id, *callback = (PyObject *)private_data; + PyObject *py_server_id, *callback_and_tuple = (PyObject *)private_data; + PyObject *callback, *py_private; struct server_id *p_server_id = talloc(NULL, struct server_id); if (!p_server_id) { @@ -182,10 +184,18 @@ static void py_msg_callback_wrapper(struct imessaging_context *msg, void *privat } *p_server_id = server_id; + if (!PyArg_ParseTuple(callback_and_tuple, "OO", + &callback, + &py_private)) { + return; + } + py_server_id = py_return_ndr_struct("samba.dcerpc.server_id", "server_id", p_server_id, p_server_id); talloc_unlink(NULL, p_server_id); - PyObject_CallFunction(callback, discard_const_p(char, "i(O)s#"), msg_type, + PyObject_CallFunction(callback, discard_const_p(char, "OiOs#"), + py_private, + msg_type, py_server_id, data->data, data->length); } @@ -194,24 +204,30 @@ static PyObject *py_imessaging_register(PyObject *self, PyObject *args, PyObject { imessaging_Object *iface = (imessaging_Object *)self; int msg_type = -1; - PyObject *callback; + PyObject *callback_and_context; NTSTATUS status; - const char *kwnames[] = { "callback", "msg_type", NULL }; + const char *kwnames[] = { "callback_and_context", "msg_type", NULL }; if (!PyArg_ParseTupleAndKeywords(args, kwargs, "O|i:register", - discard_const_p(char *, kwnames), &callback, &msg_type)) { + discard_const_p(char *, kwnames), + &callback_and_context, &msg_type)) { + return NULL; + } + if (!PyTuple_Check(callback_and_context) + || PyTuple_Size(callback_and_context) != 2) { + PyErr_SetString(PyExc_ValueError, "Expected of size 2 for callback_and_context"); return NULL; } - Py_INCREF(callback); + Py_INCREF(callback_and_context); if (msg_type == -1) { uint32_t msg_type32 = msg_type; - status = imessaging_register_tmp(iface->msg_ctx, callback, + status = imessaging_register_tmp(iface->msg_ctx, callback_and_context, py_msg_callback_wrapper, &msg_type32); msg_type = msg_type32; } else { - status = imessaging_register(iface->msg_ctx, callback, + status = imessaging_register(iface->msg_ctx, callback_and_context, msg_type, py_msg_callback_wrapper); } if (NT_STATUS_IS_ERR(status)) { @@ -241,6 +257,52 @@ static PyObject *py_imessaging_deregister(PyObject *self, PyObject *args, PyObje Py_RETURN_NONE; } +static void simple_timer_handler(struct tevent_context *ev, + struct tevent_timer *te, + struct timeval current_time, + void *private_data) +{ + return; +} + +static PyObject *py_imessaging_loop_once(PyObject *self, PyObject *args, PyObject *kwargs) +{ + imessaging_Object *iface = (imessaging_Object *)self; + double offset; + int seconds; + struct timeval next_event; + struct tevent_timer *timer = NULL; + const char *kwnames[] = { "timeout", NULL }; + + TALLOC_CTX *frame = talloc_stackframe(); + + if (!PyArg_ParseTupleAndKeywords(args, kwargs, "d", + discard_const_p(char *, kwnames), &offset)) { + TALLOC_FREE(frame); + return NULL; + } + + if (offset != 0.0) { + seconds = offset; + offset -= seconds; + next_event = tevent_timeval_current_ofs(seconds, (int)(offset*1000000)); + + timer = tevent_add_timer(iface->msg_ctx->ev, frame, next_event, simple_timer_handler, + NULL); + if (timer == NULL) { + PyErr_NoMemory(); + TALLOC_FREE(frame); + return NULL; + } + } + + tevent_loop_once(iface->msg_ctx->ev); + + TALLOC_FREE(frame); + + Py_RETURN_NONE; +} + static PyObject *py_irpc_add_name(PyObject *self, PyObject *args, PyObject *kwargs) { imessaging_Object *iface = (imessaging_Object *)self; @@ -374,6 +436,8 @@ static PyMethodDef py_imessaging_methods[] = { "S.register(callback, msg_type=None) -> msg_type\nRegister a message handler" }, { "deregister", (PyCFunction)py_imessaging_deregister, METH_VARARGS|METH_KEYWORDS, "S.deregister(callback, msg_type) -> None\nDeregister a message handler" }, + { "loop_once", (PyCFunction)py_imessaging_loop_once, METH_VARARGS|METH_KEYWORDS, + "S.loop_once(timeout) -> None\nLoop on the internal event context until we get an event (which might be a message calling the callback), timeout after timeout seconds (if not 0)" }, { "irpc_add_name", (PyCFunction)py_irpc_add_name, METH_VARARGS, "S.irpc_add_name(name) -> None\nAdd this context to the list of server_id values that are registered for a particular name" }, { "irpc_remove_name", (PyCFunction)py_irpc_remove_name, METH_VARARGS, -- 2.9.3 From 02d35eda70ea0ad8bf097f4dec32470bf55a0e66 Mon Sep 17 00:00:00 2001 From: Gary Lockyer Date: Thu, 16 Mar 2017 16:26:01 +1300 Subject: [PATCH 7/7] pymessaging: add single element tupple form of the server_id This avoids the python code needing to call getpid() internally, while declaring a stable task_id. Signed-off-by: Gary Lockyer --- python/samba/tests/messaging.py | 42 +++++++++++++++++++++++++++++++++++++ source4/lib/messaging/pymessaging.c | 9 +++++++- 2 files changed, 50 insertions(+), 1 deletion(-) diff --git a/python/samba/tests/messaging.py b/python/samba/tests/messaging.py index 6ee18e7..e9e6bf1 100644 --- a/python/samba/tests/messaging.py +++ b/python/samba/tests/messaging.py @@ -25,6 +25,7 @@ import time from samba.ndr import ndr_print from samba.dcerpc import server_id import random +import os class MessagingTests(TestCase): @@ -109,3 +110,44 @@ class MessagingTests(TestCase): self.assertEqual(got_ping["count"], 2) self.assertEqual(got_pong["count"], 2) + + def test_pid_defaulting(self): + got_ping = {"count": 0} + got_pong = {"count": 0} + timeout = False + + msg_pong = 0 + msg_ping = 0 + + pid = os.getpid() + server_ctx = self.get_context((pid, 1)) + def ping_callback(got_ping, msg_type, src, data): + got_ping["count"] += 1 + server_ctx.send(src, msg_pong, data) + + msg_ping = server_ctx.register((ping_callback, got_ping)) + + def pong_callback(got_pong, msg_type, src, data): + got_pong["count"] += 1 + + client_ctx = self.get_context((2,)) + msg_pong = client_ctx.register((pong_callback, got_pong)) + + # Try 1 an two element tuple forms + client_ctx.send((pid, 1), msg_ping, "testing") + + client_ctx.send((1,), msg_ping, "testing2") + + start_time = time.time() + + # NOTE WELL: If debugging this with GDB, then the timeout will + # fire while you are trying to understand it. + + while (got_ping["count"] < 2 or got_pong["count"] < 2) and not timeout: + client_ctx.loop_once(0.1) + server_ctx.loop_once(0.1) + if time.time() - start_time > 1: + timeout = True + + self.assertEqual(got_ping["count"], 2) + self.assertEqual(got_pong["count"], 2) diff --git a/source4/lib/messaging/pymessaging.c b/source4/lib/messaging/pymessaging.c index 84ceff5..54920dd 100644 --- a/source4/lib/messaging/pymessaging.c +++ b/source4/lib/messaging/pymessaging.c @@ -62,13 +62,20 @@ static bool server_id_from_py(PyObject *object, struct server_id *server_id) server_id->task_id = task_id; server_id->vnn = vnn; return true; - } else { + } else if (PyTuple_Size(object) == 2) { unsigned long long pid; int task_id; if (!PyArg_ParseTuple(object, "KI", &pid, &task_id)) return false; *server_id = cluster_id(pid, task_id); return true; + } else { + unsigned long long pid = getpid(); + int task_id; + if (!PyArg_ParseTuple(object, "I", &task_id)) + return false; + *server_id = cluster_id(pid, task_id); + return true; } } -- 2.9.3