From 3ec8e576d99a332514a5338671a18413ce03ba98 Mon Sep 17 00:00:00 2001 From: Martin Hurton Date: Tue, 12 Jun 2012 01:39:16 +0200 Subject: [PATCH] Fix race conditions in {tcp,ipc}_connecter Once the object has been terminated, it is unsafe for this object to refer to its parent. The bug was responsible for occasional test_shutdown_stress failures. --- src/ipc_connecter.cpp | 28 +++++++++++++++++++++------- src/ipc_connecter.hpp | 1 + src/ipc_listener.cpp | 3 +-- src/tcp_connecter.cpp | 28 +++++++++++++++++++++------- src/tcp_connecter.hpp | 1 + src/tcp_listener.cpp | 3 +-- 6 files changed, 46 insertions(+), 18 deletions(-) diff --git a/src/ipc_connecter.cpp b/src/ipc_connecter.cpp index df3993dd..818ff865 100644 --- a/src/ipc_connecter.cpp +++ b/src/ipc_connecter.cpp @@ -59,13 +59,9 @@ zmq::ipc_connecter_t::ipc_connecter_t (class io_thread_t *io_thread_, zmq::ipc_connecter_t::~ipc_connecter_t () { - if (wait) - cancel_timer (reconnect_timer_id); - if (handle_valid) - rm_fd (handle); - - if (s != retired_fd) - close (); + zmq_assert (!wait); + zmq_assert (!handle_valid); + zmq_assert (s == retired_fd); } void zmq::ipc_connecter_t::process_plug () @@ -76,6 +72,24 @@ void zmq::ipc_connecter_t::process_plug () start_connecting (); } +void zmq::ipc_connecter_t::process_term (int linger_) +{ + if (wait) { + cancel_timer (reconnect_timer_id); + wait = false; + } + + if (handle_valid) { + rm_fd (handle); + handle_valid = false; + } + + if (s != retired_fd) + close (); + + own_t::process_term (linger_); +} + void zmq::ipc_connecter_t::in_event () { // We are not polling for incomming data, so we are actually called diff --git a/src/ipc_connecter.hpp b/src/ipc_connecter.hpp index 34df9776..604922b8 100644 --- a/src/ipc_connecter.hpp +++ b/src/ipc_connecter.hpp @@ -55,6 +55,7 @@ namespace zmq // Handlers for incoming commands. void process_plug (); + void process_term (int linger_); // Handlers for I/O events. void in_event (); diff --git a/src/ipc_listener.cpp b/src/ipc_listener.cpp index becf30dc..0033eef4 100644 --- a/src/ipc_listener.cpp +++ b/src/ipc_listener.cpp @@ -52,8 +52,7 @@ zmq::ipc_listener_t::ipc_listener_t (io_thread_t *io_thread_, zmq::ipc_listener_t::~ipc_listener_t () { - if (s != retired_fd) - close (); + zmq_assert (s == retired_fd); } void zmq::ipc_listener_t::process_plug () diff --git a/src/tcp_connecter.cpp b/src/tcp_connecter.cpp index 407490c3..0a6fe128 100644 --- a/src/tcp_connecter.cpp +++ b/src/tcp_connecter.cpp @@ -69,13 +69,9 @@ zmq::tcp_connecter_t::tcp_connecter_t (class io_thread_t *io_thread_, zmq::tcp_connecter_t::~tcp_connecter_t () { - if (wait) - cancel_timer (reconnect_timer_id); - if (handle_valid) - rm_fd (handle); - - if (s != retired_fd) - close (); + zmq_assert (!wait); + zmq_assert (!handle_valid); + zmq_assert (s == retired_fd); } void zmq::tcp_connecter_t::process_plug () @@ -86,6 +82,24 @@ void zmq::tcp_connecter_t::process_plug () start_connecting (); } +void zmq::tcp_connecter_t::process_term (int linger_) +{ + if (wait) { + cancel_timer (reconnect_timer_id); + wait = false; + } + + if (handle_valid) { + rm_fd (handle); + handle_valid = false; + } + + if (s != retired_fd) + close (); + + own_t::process_term (linger_); +} + void zmq::tcp_connecter_t::in_event () { // We are not polling for incomming data, so we are actually called diff --git a/src/tcp_connecter.hpp b/src/tcp_connecter.hpp index a157dc55..e1b26b23 100644 --- a/src/tcp_connecter.hpp +++ b/src/tcp_connecter.hpp @@ -53,6 +53,7 @@ namespace zmq // Handlers for incoming commands. void process_plug (); + void process_term (int linger_); // Handlers for I/O events. void in_event (); diff --git a/src/tcp_listener.cpp b/src/tcp_listener.cpp index 173a22f1..5fbe3cd9 100644 --- a/src/tcp_listener.cpp +++ b/src/tcp_listener.cpp @@ -61,8 +61,7 @@ zmq::tcp_listener_t::tcp_listener_t (io_thread_t *io_thread_, zmq::tcp_listener_t::~tcp_listener_t () { - if (s != retired_fd) - close (); + zmq_assert (s == retired_fd); } void zmq::tcp_listener_t::process_plug ()