PROTON-1628: [cpp] Stopping container in on_container_start will hang Check if already stopping before entering the event loop in container::impl::thread()
Project: http://git-wip-us.apache.org/repos/asf/qpid-proton/repo Commit: http://git-wip-us.apache.org/repos/asf/qpid-proton/commit/e1708890 Tree: http://git-wip-us.apache.org/repos/asf/qpid-proton/tree/e1708890 Diff: http://git-wip-us.apache.org/repos/asf/qpid-proton/diff/e1708890 Branch: refs/heads/go1 Commit: e1708890cad88175f5465275a854bd279b7109bc Parents: d09511f Author: Alan Conway <acon...@redhat.com> Authored: Mon Oct 23 17:24:10 2017 +0100 Committer: Alan Conway <acon...@redhat.com> Committed: Mon Oct 23 17:24:10 2017 +0100 ---------------------------------------------------------------------- proton-c/bindings/cpp/src/container_test.cpp | 30 +++++++--- .../cpp/src/proactor_container_impl.cpp | 63 ++++++++++++-------- 2 files changed, 59 insertions(+), 34 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/e1708890/proton-c/bindings/cpp/src/container_test.cpp ---------------------------------------------------------------------- diff --git a/proton-c/bindings/cpp/src/container_test.cpp b/proton-c/bindings/cpp/src/container_test.cpp index fc368d0..c0a9734 100644 --- a/proton-c/bindings/cpp/src/container_test.cpp +++ b/proton-c/bindings/cpp/src/container_test.cpp @@ -245,17 +245,31 @@ int test_container_schedule_nohang() { return 0; } +class immediate_stop_tester : public proton::messaging_handler { +public: + void on_container_start(proton::container &c) PN_CPP_OVERRIDE { + c.stop(); + } +}; + +int test_container_immediate_stop() { + immediate_stop_tester t; + proton::container(t).run(); // will hang + return 0; } -int main(int, char**) { +} // namespace + +int main(int argc, char** argv) { int failed = 0; - RUN_TEST(failed, test_container_default_container_id()); - RUN_TEST(failed, test_container_vhost()); - RUN_TEST(failed, test_container_default_vhost()); - RUN_TEST(failed, test_container_no_vhost()); - RUN_TEST(failed, test_container_bad_address()); - RUN_TEST(failed, test_container_stop()); - RUN_TEST(failed, test_container_schedule_nohang()); + RUN_ARGV_TEST(failed, test_container_default_container_id()); + RUN_ARGV_TEST(failed, test_container_vhost()); + RUN_ARGV_TEST(failed, test_container_default_vhost()); + RUN_ARGV_TEST(failed, test_container_no_vhost()); + RUN_ARGV_TEST(failed, test_container_bad_address()); + RUN_ARGV_TEST(failed, test_container_stop()); + RUN_ARGV_TEST(failed, test_container_schedule_nohang()); + RUN_ARGV_TEST(failed, test_container_immediate_stop()); return failed; } http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/e1708890/proton-c/bindings/cpp/src/proactor_container_impl.cpp ---------------------------------------------------------------------- diff --git a/proton-c/bindings/cpp/src/proactor_container_impl.cpp b/proton-c/bindings/cpp/src/proactor_container_impl.cpp index 0965391..427bb7c 100644 --- a/proton-c/bindings/cpp/src/proactor_container_impl.cpp +++ b/proton-c/bindings/cpp/src/proactor_container_impl.cpp @@ -455,6 +455,7 @@ void container::impl::run_timer_jobs() { } } +// Return true if this thread is finished bool container::impl::handle(pn_event_t* event) { // If we have any pending connection work, do it now @@ -620,32 +621,38 @@ bool container::impl::handle(pn_event_t* event) { } void container::impl::thread() { + bool finished; { GUARD(lock_); ++threads_; + finished = stopping_; } - bool finished = false; - do { - pn_event_batch_t *events = pn_proactor_wait(proactor_); - pn_event_t *e; - try { - while ((e = pn_event_batch_next(events))) { - finished = handle(e); - if (finished) break; + while (!finished) { + pn_event_batch_t *events = pn_proactor_wait(proactor_); + pn_event_t *e; + const char *what = 0; + try { + while ((e = pn_event_batch_next(events))) { + finished = handle(e); + if (finished) break; + } + } catch (const std::exception& e) { + // If we caught an exception then shutdown the (other threads of the) container + what = e.what(); + } catch (...) { + what = "container shut-down by unknown exception"; + } + pn_proactor_done(proactor_, events); + if (what) { + finished = true; + error_condition error("exception", what); + { + GUARD(lock_); + disconnect_error_ = error; + } + stop(error); } - } catch (const std::exception& e) { - // If we caught an exception then shutdown the (other threads of the) container - disconnect_error_ = error_condition("exception", e.what()); - if (!stopping_) stop(disconnect_error_); - finished = true; - } catch (...) { - // If we caught an exception then shutdown the (other threads of the) container - disconnect_error_ = error_condition("exception", "container shut-down by unknown exception"); - if (!stopping_) stop(disconnect_error_); - finished = true; - } - pn_proactor_done(proactor_, events); - } while(!finished); + } { GUARD(lock_); --threads_; @@ -692,8 +699,9 @@ void container::impl::run(int threads) { if (last) CALL_ONCE(stop_once_, &impl::stop_event, this); // Throw an exception if we disconnected the proactor because of an exception - if (!disconnect_error_.empty()) { - throw proton::error(disconnect_error_.description()); + { + GUARD(lock_); + if (!disconnect_error_.empty()) throw proton::error(disconnect_error_.description()); }; } @@ -703,9 +711,12 @@ void container::impl::auto_stop(bool set) { } void container::impl::stop(const proton::error_condition& err) { - GUARD(lock_); - auto_stop_ = true; - stopping_ = true; + { + GUARD(lock_); + if (stopping_) return; // Already stopping + auto_stop_ = true; + stopping_ = true; + } pn_condition_t* error_condition = pn_condition(); set_error_condition(err, error_condition); pn_proactor_disconnect(proactor_, error_condition); --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@qpid.apache.org For additional commands, e-mail: commits-h...@qpid.apache.org