You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@qpid.apache.org by ac...@apache.org on 2017/11/15 21:05:34 UTC

[10/31] qpid-proton git commit: PROTON-1628: [cpp] Stopping container in on_container_start will hang

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 <ac...@redhat.com>
Authored: Mon Oct 23 17:24:10 2017 +0100
Committer: Alan Conway <ac...@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-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org