From 32c8abb1d88e3c328a6045ca1f6298d57bdfdd7f Mon Sep 17 00:00:00 2001 From: Simon Giesecke Date: Wed, 28 Mar 2018 09:50:58 +0200 Subject: [PATCH 1/3] Problem: regression when zmq_poller_destroy is called after zmq_ctx_term Solution: Added test case to reproduce, not solving the problem! --- tests/test_ctx_destroy.cpp | 66 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/tests/test_ctx_destroy.cpp b/tests/test_ctx_destroy.cpp index b708758e..4998e33d 100644 --- a/tests/test_ctx_destroy.cpp +++ b/tests/test_ctx_destroy.cpp @@ -112,6 +112,69 @@ void test_zmq_ctx_shutdown_null_fails () TEST_ASSERT_EQUAL_INT (EFAULT, errno); } +#ifdef ZMQ_HAVE_POLLER +struct poller_test_data_t +{ + void *ctx; + void *counter; +}; + +void run_poller (void *data_) +{ + struct poller_test_data_t *poller_test_data = + (struct poller_test_data_t *) data_; + + void *socket = zmq_socket (poller_test_data->ctx, ZMQ_PULL); + TEST_ASSERT_NOT_NULL (socket); + + void *poller = zmq_poller_new (); + TEST_ASSERT_NOT_NULL (poller); + + TEST_ASSERT_SUCCESS_ERRNO ( + zmq_poller_add (poller, socket, NULL, ZMQ_POLLIN)); + + zmq_atomic_counter_set (poller_test_data->counter, 1); + + zmq_poller_event_t event; + TEST_ASSERT_FAILURE_ERRNO (ETERM, zmq_poller_wait (poller, &event, -1)); + + TEST_ASSERT_SUCCESS_ERRNO (zmq_poller_destroy (&poller)); + + // Close the socket + TEST_ASSERT_SUCCESS_ERRNO (zmq_close (socket)); +} +#endif + +void test_poller_exists_with_socket_on_zmq_ctx_term () +{ +#ifdef ZMQ_HAVE_POLLER + struct poller_test_data_t poller_test_data; + + // Set up our context and sockets + poller_test_data.ctx = zmq_ctx_new (); + TEST_ASSERT_NOT_NULL (poller_test_data.ctx); + + poller_test_data.counter = zmq_atomic_counter_new (); + TEST_ASSERT_NOT_NULL (poller_test_data.counter); + + void *thread = zmq_threadstart (run_poller, &poller_test_data); + TEST_ASSERT_NOT_NULL (thread); + + while (zmq_atomic_counter_value (poller_test_data.counter) == 0) { + msleep (10); + } + + // Destroy the context + TEST_ASSERT_SUCCESS_ERRNO (zmq_ctx_destroy (poller_test_data.ctx)); + + zmq_threadclose (thread); + + zmq_atomic_counter_destroy (&poller_test_data.counter); +#else + TEST_IGNORE_MESSAGE ("libzmq without zmq_poller_* support, ignoring test"); +#endif +} + int main (void) { setup_test_environment (); @@ -122,5 +185,8 @@ int main (void) RUN_TEST (test_zmq_ctx_term_null_fails); RUN_TEST (test_zmq_term_null_fails); RUN_TEST (test_zmq_ctx_shutdown_null_fails); + + RUN_TEST (test_poller_exists_with_socket_on_zmq_ctx_term); + return UNITY_END (); } From 87fbb5c4478dda2cdcb7e78954c17c462b467438 Mon Sep 17 00:00:00 2001 From: Simon Giesecke Date: Wed, 28 Mar 2018 10:46:19 +0200 Subject: [PATCH 2/3] Problem: socket poller shutdown asserts when context is terminating Solution: do not call getsockopt to query thread-safety of a socket --- src/socket_base.cpp | 5 +++++ src/socket_base.hpp | 3 +++ src/socket_poller.cpp | 9 ++------- 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/src/socket_base.cpp b/src/socket_base.cpp index 70fef8b1..334e05ec 100644 --- a/src/socket_base.cpp +++ b/src/socket_base.cpp @@ -102,6 +102,11 @@ bool zmq::socket_base_t::check_tag () return tag == 0xbaddecaf; } +bool zmq::socket_base_t::is_thread_safe () const +{ + return thread_safe; +} + zmq::socket_base_t *zmq::socket_base_t::create (int type_, class ctx_t *parent_, uint32_t tid_, diff --git a/src/socket_base.hpp b/src/socket_base.hpp index 06cf1b06..5655819c 100644 --- a/src/socket_base.hpp +++ b/src/socket_base.hpp @@ -67,6 +67,9 @@ class socket_base_t : public own_t, // Returns false if object is not a socket. bool check_tag (); + // Returns whether the socket is thread-safe. + bool is_thread_safe () const; + // Create a socket of a specified type. static socket_base_t * create (int type_, zmq::ctx_t *parent_, uint32_t tid_, int sid_); diff --git a/src/socket_poller.cpp b/src/socket_poller.cpp index e45bec9e..570e4b8d 100644 --- a/src/socket_poller.cpp +++ b/src/socket_poller.cpp @@ -33,13 +33,8 @@ static bool is_thread_safe (zmq::socket_base_t &socket) { - int thread_safe; - size_t thread_safe_size = sizeof (int); - - int rc = - socket.getsockopt (ZMQ_THREAD_SAFE, &thread_safe, &thread_safe_size); - zmq_assert (rc == 0); - return thread_safe; + // do not use getsockopt here, since that would fail during context termination + return socket.is_thread_safe (); } zmq::socket_poller_t::socket_poller_t () : From f571c22851d3a2f2d91def24410a44d07a913ace Mon Sep 17 00:00:00 2001 From: Simon Giesecke Date: Wed, 28 Mar 2018 10:47:12 +0200 Subject: [PATCH 3/3] Problem: socket_poller destruction after context shutdown is only tested with a non-thread-safe socket Solution: test with both thread-safe and non-thread-safe sockets --- tests/test_ctx_destroy.cpp | 27 ++++++++++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/tests/test_ctx_destroy.cpp b/tests/test_ctx_destroy.cpp index 4998e33d..1ce04ded 100644 --- a/tests/test_ctx_destroy.cpp +++ b/tests/test_ctx_destroy.cpp @@ -115,6 +115,7 @@ void test_zmq_ctx_shutdown_null_fails () #ifdef ZMQ_HAVE_POLLER struct poller_test_data_t { + int socket_type; void *ctx; void *counter; }; @@ -124,7 +125,8 @@ void run_poller (void *data_) struct poller_test_data_t *poller_test_data = (struct poller_test_data_t *) data_; - void *socket = zmq_socket (poller_test_data->ctx, ZMQ_PULL); + void *socket = + zmq_socket (poller_test_data->ctx, poller_test_data->socket_type); TEST_ASSERT_NOT_NULL (socket); void *poller = zmq_poller_new (); @@ -145,11 +147,13 @@ void run_poller (void *data_) } #endif -void test_poller_exists_with_socket_on_zmq_ctx_term () +void test_poller_exists_with_socket_on_zmq_ctx_term (const int socket_type) { #ifdef ZMQ_HAVE_POLLER struct poller_test_data_t poller_test_data; + poller_test_data.socket_type = socket_type; + // Set up our context and sockets poller_test_data.ctx = zmq_ctx_new (); TEST_ASSERT_NOT_NULL (poller_test_data.ctx); @@ -175,6 +179,20 @@ void test_poller_exists_with_socket_on_zmq_ctx_term () #endif } +void test_poller_exists_with_socket_on_zmq_ctx_term_thread_safe_socket () +{ +#ifdef ZMQ_BUILD_DRAFT_API + test_poller_exists_with_socket_on_zmq_ctx_term (ZMQ_CLIENT); +#else + TEST_IGNORE_MESSAGE ("libzmq without DRAFT support, ignoring test"); +#endif +} + +void test_poller_exists_with_socket_on_zmq_ctx_term_non_thread_safe_socket () +{ + test_poller_exists_with_socket_on_zmq_ctx_term (ZMQ_DEALER); +} + int main (void) { setup_test_environment (); @@ -186,7 +204,10 @@ int main (void) RUN_TEST (test_zmq_term_null_fails); RUN_TEST (test_zmq_ctx_shutdown_null_fails); - RUN_TEST (test_poller_exists_with_socket_on_zmq_ctx_term); + RUN_TEST ( + test_poller_exists_with_socket_on_zmq_ctx_term_non_thread_safe_socket); + RUN_TEST ( + test_poller_exists_with_socket_on_zmq_ctx_term_thread_safe_socket); return UNITY_END (); }