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/09/13 17:20:47 UTC

[1/2] qpid-proton git commit: PROTON-1586: c proactor example clients core dump with bad host

Repository: qpid-proton
Updated Branches:
  refs/heads/master c6d0860f7 -> cf5e939a9


PROTON-1586: c proactor example clients core dump with bad host

Fixed crash: move timer rearm logic to after check for connection closed,
so timer is not rearmed for a closed connection.

Fixed events after PN_TRANSPORT_CLOSE


Project: http://git-wip-us.apache.org/repos/asf/qpid-proton/repo
Commit: http://git-wip-us.apache.org/repos/asf/qpid-proton/commit/cf5e939a
Tree: http://git-wip-us.apache.org/repos/asf/qpid-proton/tree/cf5e939a
Diff: http://git-wip-us.apache.org/repos/asf/qpid-proton/diff/cf5e939a

Branch: refs/heads/master
Commit: cf5e939a9d3d48edc1858bbab3f9316bf84375e9
Parents: 73bb4da
Author: Alan Conway <ac...@redhat.com>
Authored: Wed Sep 13 12:21:04 2017 -0400
Committer: Alan Conway <ac...@redhat.com>
Committed: Wed Sep 13 13:03:18 2017 -0400

----------------------------------------------------------------------
 proton-c/include/proton/connection_driver.h |  2 +-
 proton-c/src/core/connection_driver.c       | 14 +++++++--
 proton-c/src/proactor/epoll.c               | 30 ++++++++++----------
 proton-c/src/tests/proactor.c               | 36 ++++++++++++++++++++++--
 4 files changed, 62 insertions(+), 20 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/cf5e939a/proton-c/include/proton/connection_driver.h
----------------------------------------------------------------------
diff --git a/proton-c/include/proton/connection_driver.h b/proton-c/include/proton/connection_driver.h
index 82c7eb1..89040af 100644
--- a/proton-c/include/proton/connection_driver.h
+++ b/proton-c/include/proton/connection_driver.h
@@ -191,7 +191,7 @@ PN_EXTERN void pn_connection_driver_write_close(pn_connection_driver_t *);
 PN_EXTERN bool pn_connection_driver_write_closed(pn_connection_driver_t *);
 
 /**
- * Close both sides side.
+ * Close both sides.
  */
 PN_EXTERN void pn_connection_driver_close(pn_connection_driver_t * c);
 

http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/cf5e939a/proton-c/src/core/connection_driver.c
----------------------------------------------------------------------
diff --git a/proton-c/src/core/connection_driver.c b/proton-c/src/core/connection_driver.c
index 0c520da..eb2df5f 100644
--- a/proton-c/src/core/connection_driver.c
+++ b/proton-c/src/core/connection_driver.c
@@ -34,9 +34,19 @@ static pn_event_t *batch_next(pn_event_batch_t *batch) {
   pn_connection_driver_t *d =
     (pn_connection_driver_t*)((char*)batch - offsetof(pn_connection_driver_t, batch));
   pn_event_t *handled = pn_collector_prev(d->collector);
-  if (handled && pn_event_type(handled) == PN_CONNECTION_INIT) {
-      pn_transport_bind(d->transport, d->connection); /* Init event handled, auto-bind */
+  if (handled) {
+    switch (pn_event_type(handled)) {
+     case PN_CONNECTION_INIT:   /* Auto-bind after the INIT event is handled */
+      pn_transport_bind(d->transport, d->connection);
+      break;
+     case PN_TRANSPORT_CLOSED:  /* No more events after TRANSPORT_CLOSED  */
+      if (d->collector) pn_collector_release(d->collector);
+      break;
+     default:
+      break;
+    }
   }
+  /* Log the next event that will be processed */
   pn_event_t *next = pn_collector_next(d->collector);
   if (next && d->transport->trace & PN_TRACE_EVT) {
     pn_string_clear(d->transport->scratch);

http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/cf5e939a/proton-c/src/proactor/epoll.c
----------------------------------------------------------------------
diff --git a/proton-c/src/proactor/epoll.c b/proton-c/src/proactor/epoll.c
index 6f0cc96..c06c5d2 100644
--- a/proton-c/src/proactor/epoll.c
+++ b/proton-c/src/proactor/epoll.c
@@ -970,7 +970,7 @@ static void pconnection_maybe_connect_lh(pconnection_t *pc);
  */
 static pn_event_batch_t *pconnection_process(pconnection_t *pc, uint32_t events, bool timeout, bool topup) {
   bool inbound_wake = !(events | timeout | topup);
-  bool timer_unarmed = false;
+  bool rearm_timer = false;
   bool timer_fired = false;
   bool waking = false;
   bool tick_required = false;
@@ -978,7 +978,7 @@ static pn_event_batch_t *pconnection_process(pconnection_t *pc, uint32_t events,
   // Don't touch data exclusive to working thread (yet).
 
   if (timeout) {
-    timer_unarmed = true;
+    rearm_timer = true;
     timer_fired = ptimer_callback(&pc->timer) != 0;
   }
   lock(&pc->context.mutex);
@@ -996,7 +996,7 @@ static pn_event_batch_t *pconnection_process(pconnection_t *pc, uint32_t events,
     inbound_wake = false;
   }
 
-  if (timer_unarmed)
+  if (rearm_timer)
     pc->timer_armed = false;
 
   if (topup) {
@@ -1062,18 +1062,9 @@ static pn_event_batch_t *pconnection_process(pconnection_t *pc, uint32_t events,
     return NULL;
   }
 
-  if (!pc->timer_armed && !pc->timer.shutting_down && pc->timer.timerfd >= 0) {
-    pc->timer_armed = true;  // about to rearm outside the lock
-    timer_unarmed = true;    // so we remember
-  }
-
   unlock(&pc->context.mutex);
   pc->hog_count++; // working context doing work
 
-  if (timer_unarmed) {
-    rearm(pc->psocket.proactor, &pc->timer.epoll_io);
-    timer_unarmed = false;
-  }
   if (waking) {
     pn_connection_t *c = pc->driver.connection;
     pn_collector_put(pn_connection_collector(c), PN_OBJECT, c, PN_CONNECTION_WAKE);
@@ -1143,10 +1134,19 @@ static pn_event_batch_t *pconnection_process(pconnection_t *pc, uint32_t events,
       return NULL;
     }
   }
+  bool rearm_pc = pconnection_rearm_check(pc);
 
-  bool rearm = pconnection_rearm_check(pc);
+  if (!pc->timer_armed && !pc->timer.shutting_down && pc->timer.timerfd >= 0) {
+    pc->timer_armed = true;  // about to rearm outside the lock
+    rearm_timer = true;      // so we remember
+  }
   unlock(&pc->context.mutex);
-  if (rearm) pconnection_rearm(pc);
+
+  if (rearm_timer) {
+    rearm(pc->psocket.proactor, &pc->timer.epoll_io);
+  }
+  if (rearm_pc) pconnection_rearm(pc);
+
   return NULL;
 }
 
@@ -1244,7 +1244,7 @@ void pn_proactor_connect(pn_proactor_t *p, pn_connection_t *c, const char *addr)
   pconnection_t *pc = (pconnection_t*) pn_class_new(&pconnection_class, sizeof(pconnection_t));
   assert(pc); // TODO: memory safety
   const char *err = pconnection_setup(pc, p, c, false, addr);
-  if (err) {
+  if (err) {    /* TODO aconway 2017-09-13: errors must be reported as events */
     pn_logf("pn_proactor_connect failure: %s", err);
     return;
   }

http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/cf5e939a/proton-c/src/tests/proactor.c
----------------------------------------------------------------------
diff --git a/proton-c/src/tests/proactor.c b/proton-c/src/tests/proactor.c
index 95c6fbe..f22452e 100644
--- a/proton-c/src/tests/proactor.c
+++ b/proton-c/src/tests/proactor.c
@@ -604,9 +604,40 @@ static void test_errors(test_t *t) {
   if (TEST_ETYPE_EQUAL(t, PN_TRANSPORT_CLOSED, TEST_PROACTORS_RUN(tps))) {
     TEST_COND_DESC(t, "refused", last_condition);
     TEST_ETYPE_EQUAL(t, PN_PROACTOR_INACTIVE, TEST_PROACTORS_RUN(tps));
-    sock_close(port.sock);
-    TEST_PROACTORS_DESTROY(tps);
   }
+
+  sock_close(port.sock);
+  TEST_PROACTORS_DESTROY(tps);
+}
+
+/* Closing the connection during PN_TRANSPORT_ERROR should be a no-op
+ * Regression test for: https://issues.apache.org/jira/browse/PROTON-1586
+ */
+static pn_event_type_t transport_close_connection_handler(test_handler_t *th, pn_event_t *e) {
+  switch (pn_event_type(e)) {
+   case PN_TRANSPORT_ERROR:
+    pn_connection_close(pn_event_connection(e));
+    break;
+   default:
+    return open_wake_handler(th, e);
+  }
+  return PN_EVENT_NONE;
+}
+
+/* Closing the connection during PN_TRANSPORT_ERROR due to connection failure should be a no-op
+ * Regression test for: https://issues.apache.org/jira/browse/PROTON-1586
+ */
+static void test_proton_1586(test_t *t) {
+  test_proactor_t tps[] =  { test_proactor(t, transport_close_connection_handler) };
+  pn_proactor_connect(tps[0].proactor, pn_connection(), ":yyy");
+  TEST_ETYPE_EQUAL(t, PN_TRANSPORT_CLOSED, TEST_PROACTORS_RUN(tps));
+  TEST_COND_DESC(t, ":yyy", last_condition);
+  test_handler_keep(&tps[0].handler, 0); /* Clear events */
+  /* There should be no events generated after PN_TRANSPORT_CLOSED */
+  TEST_ETYPE_EQUAL(t, PN_PROACTOR_INACTIVE, TEST_PROACTORS_RUN(tps));
+  TEST_HANDLER_EXPECT(&tps[0].handler,PN_PROACTOR_INACTIVE, 0);
+
+  TEST_PROACTORS_DESTROY(tps);
 }
 
 /* Test that we can control listen/select on ipv6/v4 and listen on both by default */
@@ -1062,6 +1093,7 @@ int main(int argc, char **argv) {
   RUN_ARGV_TEST(failed, t, test_inactive(&t));
   RUN_ARGV_TEST(failed, t, test_interrupt_timeout(&t));
   RUN_ARGV_TEST(failed, t, test_errors(&t));
+  RUN_ARGV_TEST(failed, t, test_proton_1586(&t));
   RUN_ARGV_TEST(failed, t, test_client_server(&t));
   RUN_ARGV_TEST(failed, t, test_connection_wake(&t));
   RUN_ARGV_TEST(failed, t, test_ipv4_ipv6(&t));


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org


[2/2] qpid-proton git commit: PROTON-1512: Remove stray FIXME comment

Posted by ac...@apache.org.
PROTON-1512: Remove stray FIXME comment


Project: http://git-wip-us.apache.org/repos/asf/qpid-proton/repo
Commit: http://git-wip-us.apache.org/repos/asf/qpid-proton/commit/73bb4da6
Tree: http://git-wip-us.apache.org/repos/asf/qpid-proton/tree/73bb4da6
Diff: http://git-wip-us.apache.org/repos/asf/qpid-proton/diff/73bb4da6

Branch: refs/heads/master
Commit: 73bb4da612866c5ca5963fa3b87a951bad5e0947
Parents: c6d0860
Author: Alan Conway <ac...@redhat.com>
Authored: Wed Sep 13 11:53:15 2017 -0400
Committer: Alan Conway <ac...@redhat.com>
Committed: Wed Sep 13 13:03:18 2017 -0400

----------------------------------------------------------------------
 proton-c/include/proton/delivery.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/73bb4da6/proton-c/include/proton/delivery.h
----------------------------------------------------------------------
diff --git a/proton-c/include/proton/delivery.h b/proton-c/include/proton/delivery.h
index b9f627b..bab7c5c 100644
--- a/proton-c/include/proton/delivery.h
+++ b/proton-c/include/proton/delivery.h
@@ -196,7 +196,7 @@ PN_EXTERN bool pn_delivery_partial(pn_delivery_t *delivery);
  * @param[in] delivery a delivery object
  * @return true if the delivery has been aborted, false otherwise
  */
-PN_EXTERN bool pn_delivery_aborted(pn_delivery_t *delivery); /* FIXME aconway 2017-09-11:  */
+PN_EXTERN bool pn_delivery_aborted(pn_delivery_t *delivery);
 
 /**
  * Check if a delivery is writable.


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org