[jira] [Commented] (DISPATCH-2046) Panic in handle due to deleted connector.

2021-05-07 Thread ASF subversion and git services (Jira)


[ 
https://issues.apache.org/jira/browse/DISPATCH-2046?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17340793#comment-17340793
 ] 

ASF subversion and git services commented on DISPATCH-2046:
---

Commit e4ebecfa27f98fd62a78fb2991076c23a2cc2362 in qpid-dispatch's branch 
refs/heads/1.16.x from Ken Giusti
[ https://gitbox.apache.org/repos/asf?p=qpid-dispatch.git;h=e4ebecf ]

DISPATCH-1679: Make qd_timer_cancel and qd_timer_free synchronous

This patch makes the qd_timer_cancel() and qd_timer_free() routines
block if the timer callback is currently running on another thread.
This ensures that the callback is not executing when these calls
return.

It includes a fix to the HTTP1 adapter to avoid freeing its timer
while holding the adaptor lock.

This closes #1170

(cherry picked from commit 34b3c116dc97f9a1d7caf00fcea0e5e5144e6c45)

Needed by fix for blocker JIRA DISPATCH-2046


> Panic in handle due to deleted connector.
> -
>
> Key: DISPATCH-2046
> URL: https://issues.apache.org/jira/browse/DISPATCH-2046
> Project: Qpid Dispatch
>  Issue Type: Bug
>  Components: Routing Engine
>Affects Versions: 1.15.0
>Reporter: Alex Ward
>Assignee: Ken Giusti
>Priority: Blocker
> Fix For: 1.16.0
>
>
> I am seeing a segv at line 1063 due to ctx->connector being 0 in 
> server.c:handle(). 
> {code:java}
> (ugdb-x86_64-7.11-261) p ctx
> $6 = (qd_connection_t *) 0x802e55890
> (ugdb-x86_64-7.11-261) p *ctx
> $7 = {prev = 0x8023df490, next = 0x802e7a890, name = 0x0, server = 
> 0x80220d0c0, opened = false, closed = false, closed_locally = false, enqueued 
> = 0, timer = 0x0, pn_conn = 0x8023ec050, pn_sessions = {0x0  times>}, ssl = 0x0, listener = 0x0, connector = 0x0, connector_lock = 
> 0x8023e6600, context = 0x0, user_context = 0x0, link_context = 0x0, 
> connection_id = 12, user_id = 0x0, free_user_id = false, policy_settings = 
> 0x0, n_sessions = 0, n_senders = 0, n_receivers = 0, open_container = 0x0, 
> deferred_calls = {head = 0x0, tail = 0x0, scratch = 0x0, size = 0}, 
> deferred_call_lock = 0x8023e65c0, policy_counted = false, role = 0x801a17660 
> "route-container", free_link_session_list = {head = 0x0, tail = 0x0, scratch 
> = 0x0, size = 0}, strip_annotations_in = false, strip_annotations_out = 
> false, wake = 0x8005d6e20 , rhost = '\000'  times>, rhost_port = '\000' }
> {code}
> As the comment at the start of handle() states, there is only one event being 
> handled at a time, but it doesn’t look like there is anything to stop someone 
> clearing ctx->connector between lines 1055 and 1063. The python code seems to 
> be coming in and deleting the connector while handle() is using it.
> [https://github.com/apache/qpid-dispatch/blob/main/src/server.c]
> {code:java}
>  989 /* Events involving a connection or listener are serialized by the 
> proactor so
>  990  * only one event per connection / listener will be processed at a time. 
>  
>  991  */  
>  992 static bool handle(qd_server_t *qd_server, pn_event_t *e, 
> pn_connection_t *pn_conn, qd_connection_t *ctx) 
>  993 {
>  
>  994 if (pn_conn && qdr_is_authentication_service_connection(pn_conn)) {  
>  
>  995 qdr_handle_authentication_service_connection_event(e);   
>  
>  996 return true; 
>  
>  997 }
>  
>  998  
>   
>  999 switch (pn_event_type(e)) { 
>  …
> 1051 case PN_TRANSPORT_ERROR: 
>  
> 1052 {
>  
> 1053 pn_transport_t *transport = pn_event_transport(e);   
>  
> 1054 pn_condition_t* condition = transport ? 
> pn_transport_condition(transport) : NULL; 
> 1055 if (ctx && ctx->connector) { /* Outgoing connection */   
>  
> 1056 qd_increment_conn_index(ctx);
>  
> 1057 const qd_server_config_t *config = 
> &ctx->connector->config;   
> 1058 ctx->connector->state = CXTR_STATE_FAILED;   
> 

[jira] [Commented] (DISPATCH-2046) Panic in handle due to deleted connector.

2021-05-07 Thread ASF subversion and git services (Jira)


[ 
https://issues.apache.org/jira/browse/DISPATCH-2046?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17340794#comment-17340794
 ] 

ASF subversion and git services commented on DISPATCH-2046:
---

Commit dd319e527127fc2d13ad8ff11b9a07d265f2309f in qpid-dispatch's branch 
refs/heads/1.16.x from Ken Giusti
[ https://gitbox.apache.org/repos/asf?p=qpid-dispatch.git;h=dd319e5 ]

DISPATCH-2046: prevent crash when accessing connector pointer
DISPATCH-1679: fix qd_connector_t leak
DISPATCH-1917: fix race when accessing connector->conn_msg buffer

This closes #1176

(cherry picked from commit 8545a425cd8ee86beffdc97464fd4d4a20618a05)


> Panic in handle due to deleted connector.
> -
>
> Key: DISPATCH-2046
> URL: https://issues.apache.org/jira/browse/DISPATCH-2046
> Project: Qpid Dispatch
>  Issue Type: Bug
>  Components: Routing Engine
>Affects Versions: 1.15.0
>Reporter: Alex Ward
>Assignee: Ken Giusti
>Priority: Blocker
> Fix For: 1.16.0
>
>
> I am seeing a segv at line 1063 due to ctx->connector being 0 in 
> server.c:handle(). 
> {code:java}
> (ugdb-x86_64-7.11-261) p ctx
> $6 = (qd_connection_t *) 0x802e55890
> (ugdb-x86_64-7.11-261) p *ctx
> $7 = {prev = 0x8023df490, next = 0x802e7a890, name = 0x0, server = 
> 0x80220d0c0, opened = false, closed = false, closed_locally = false, enqueued 
> = 0, timer = 0x0, pn_conn = 0x8023ec050, pn_sessions = {0x0  times>}, ssl = 0x0, listener = 0x0, connector = 0x0, connector_lock = 
> 0x8023e6600, context = 0x0, user_context = 0x0, link_context = 0x0, 
> connection_id = 12, user_id = 0x0, free_user_id = false, policy_settings = 
> 0x0, n_sessions = 0, n_senders = 0, n_receivers = 0, open_container = 0x0, 
> deferred_calls = {head = 0x0, tail = 0x0, scratch = 0x0, size = 0}, 
> deferred_call_lock = 0x8023e65c0, policy_counted = false, role = 0x801a17660 
> "route-container", free_link_session_list = {head = 0x0, tail = 0x0, scratch 
> = 0x0, size = 0}, strip_annotations_in = false, strip_annotations_out = 
> false, wake = 0x8005d6e20 , rhost = '\000'  times>, rhost_port = '\000' }
> {code}
> As the comment at the start of handle() states, there is only one event being 
> handled at a time, but it doesn’t look like there is anything to stop someone 
> clearing ctx->connector between lines 1055 and 1063. The python code seems to 
> be coming in and deleting the connector while handle() is using it.
> [https://github.com/apache/qpid-dispatch/blob/main/src/server.c]
> {code:java}
>  989 /* Events involving a connection or listener are serialized by the 
> proactor so
>  990  * only one event per connection / listener will be processed at a time. 
>  
>  991  */  
>  992 static bool handle(qd_server_t *qd_server, pn_event_t *e, 
> pn_connection_t *pn_conn, qd_connection_t *ctx) 
>  993 {
>  
>  994 if (pn_conn && qdr_is_authentication_service_connection(pn_conn)) {  
>  
>  995 qdr_handle_authentication_service_connection_event(e);   
>  
>  996 return true; 
>  
>  997 }
>  
>  998  
>   
>  999 switch (pn_event_type(e)) { 
>  …
> 1051 case PN_TRANSPORT_ERROR: 
>  
> 1052 {
>  
> 1053 pn_transport_t *transport = pn_event_transport(e);   
>  
> 1054 pn_condition_t* condition = transport ? 
> pn_transport_condition(transport) : NULL; 
> 1055 if (ctx && ctx->connector) { /* Outgoing connection */   
>  
> 1056 qd_increment_conn_index(ctx);
>  
> 1057 const qd_server_config_t *config = 
> &ctx->connector->config;   
> 1058 ctx->connector->state = CXTR_STATE_FAILED;   
>  
> 1059 char conn_msg[300];  
>  
> 1060 if (condition  && pn_condition_is_set(condition)) {  
> 

[jira] [Commented] (DISPATCH-2046) Panic in handle due to deleted connector.

2021-05-03 Thread ASF subversion and git services (Jira)


[ 
https://issues.apache.org/jira/browse/DISPATCH-2046?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17338571#comment-17338571
 ] 

ASF subversion and git services commented on DISPATCH-2046:
---

Commit 8545a425cd8ee86beffdc97464fd4d4a20618a05 in qpid-dispatch's branch 
refs/heads/main from Ken Giusti
[ https://gitbox.apache.org/repos/asf?p=qpid-dispatch.git;h=8545a42 ]

DISPATCH-2046: prevent crash when accessing connector pointer
DISPATCH-1679: fix qd_connector_t leak
DISPATCH-1917: fix race when accessing connector->conn_msg buffer

This closes #1176


> Panic in handle due to deleted connector.
> -
>
> Key: DISPATCH-2046
> URL: https://issues.apache.org/jira/browse/DISPATCH-2046
> Project: Qpid Dispatch
>  Issue Type: Bug
>  Components: Routing Engine
>Affects Versions: 1.15.0
>Reporter: Alex Ward
>Assignee: Ken Giusti
>Priority: Blocker
> Fix For: 1.16.0
>
>
> I am seeing a segv at line 1063 due to ctx->connector being 0 in 
> server.c:handle(). 
> {code:java}
> (ugdb-x86_64-7.11-261) p ctx
> $6 = (qd_connection_t *) 0x802e55890
> (ugdb-x86_64-7.11-261) p *ctx
> $7 = {prev = 0x8023df490, next = 0x802e7a890, name = 0x0, server = 
> 0x80220d0c0, opened = false, closed = false, closed_locally = false, enqueued 
> = 0, timer = 0x0, pn_conn = 0x8023ec050, pn_sessions = {0x0  times>}, ssl = 0x0, listener = 0x0, connector = 0x0, connector_lock = 
> 0x8023e6600, context = 0x0, user_context = 0x0, link_context = 0x0, 
> connection_id = 12, user_id = 0x0, free_user_id = false, policy_settings = 
> 0x0, n_sessions = 0, n_senders = 0, n_receivers = 0, open_container = 0x0, 
> deferred_calls = {head = 0x0, tail = 0x0, scratch = 0x0, size = 0}, 
> deferred_call_lock = 0x8023e65c0, policy_counted = false, role = 0x801a17660 
> "route-container", free_link_session_list = {head = 0x0, tail = 0x0, scratch 
> = 0x0, size = 0}, strip_annotations_in = false, strip_annotations_out = 
> false, wake = 0x8005d6e20 , rhost = '\000'  times>, rhost_port = '\000' }
> {code}
> As the comment at the start of handle() states, there is only one event being 
> handled at a time, but it doesn’t look like there is anything to stop someone 
> clearing ctx->connector between lines 1055 and 1063. The python code seems to 
> be coming in and deleting the connector while handle() is using it.
> [https://github.com/apache/qpid-dispatch/blob/main/src/server.c]
> {code:java}
>  989 /* Events involving a connection or listener are serialized by the 
> proactor so
>  990  * only one event per connection / listener will be processed at a time. 
>  
>  991  */  
>  992 static bool handle(qd_server_t *qd_server, pn_event_t *e, 
> pn_connection_t *pn_conn, qd_connection_t *ctx) 
>  993 {
>  
>  994 if (pn_conn && qdr_is_authentication_service_connection(pn_conn)) {  
>  
>  995 qdr_handle_authentication_service_connection_event(e);   
>  
>  996 return true; 
>  
>  997 }
>  
>  998  
>   
>  999 switch (pn_event_type(e)) { 
>  …
> 1051 case PN_TRANSPORT_ERROR: 
>  
> 1052 {
>  
> 1053 pn_transport_t *transport = pn_event_transport(e);   
>  
> 1054 pn_condition_t* condition = transport ? 
> pn_transport_condition(transport) : NULL; 
> 1055 if (ctx && ctx->connector) { /* Outgoing connection */   
>  
> 1056 qd_increment_conn_index(ctx);
>  
> 1057 const qd_server_config_t *config = 
> &ctx->connector->config;   
> 1058 ctx->connector->state = CXTR_STATE_FAILED;   
>  
> 1059 char conn_msg[300];  
>  
> 1060 if (condition  && pn_condition_is_set(condition)) {  
>  
> 1061 qd_format_string(conn_msg, 300, "[C%"PR

[jira] [Commented] (DISPATCH-2046) Panic in handle due to deleted connector.

2021-05-03 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/DISPATCH-2046?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17338574#comment-17338574
 ] 

ASF GitHub Bot commented on DISPATCH-2046:
--

asfgit closed pull request #1176:
URL: https://github.com/apache/qpid-dispatch/pull/1176


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


> Panic in handle due to deleted connector.
> -
>
> Key: DISPATCH-2046
> URL: https://issues.apache.org/jira/browse/DISPATCH-2046
> Project: Qpid Dispatch
>  Issue Type: Bug
>  Components: Routing Engine
>Affects Versions: 1.15.0
>Reporter: Alex Ward
>Assignee: Ken Giusti
>Priority: Blocker
> Fix For: 1.16.0
>
>
> I am seeing a segv at line 1063 due to ctx->connector being 0 in 
> server.c:handle(). 
> {code:java}
> (ugdb-x86_64-7.11-261) p ctx
> $6 = (qd_connection_t *) 0x802e55890
> (ugdb-x86_64-7.11-261) p *ctx
> $7 = {prev = 0x8023df490, next = 0x802e7a890, name = 0x0, server = 
> 0x80220d0c0, opened = false, closed = false, closed_locally = false, enqueued 
> = 0, timer = 0x0, pn_conn = 0x8023ec050, pn_sessions = {0x0  times>}, ssl = 0x0, listener = 0x0, connector = 0x0, connector_lock = 
> 0x8023e6600, context = 0x0, user_context = 0x0, link_context = 0x0, 
> connection_id = 12, user_id = 0x0, free_user_id = false, policy_settings = 
> 0x0, n_sessions = 0, n_senders = 0, n_receivers = 0, open_container = 0x0, 
> deferred_calls = {head = 0x0, tail = 0x0, scratch = 0x0, size = 0}, 
> deferred_call_lock = 0x8023e65c0, policy_counted = false, role = 0x801a17660 
> "route-container", free_link_session_list = {head = 0x0, tail = 0x0, scratch 
> = 0x0, size = 0}, strip_annotations_in = false, strip_annotations_out = 
> false, wake = 0x8005d6e20 , rhost = '\000'  times>, rhost_port = '\000' }
> {code}
> As the comment at the start of handle() states, there is only one event being 
> handled at a time, but it doesn’t look like there is anything to stop someone 
> clearing ctx->connector between lines 1055 and 1063. The python code seems to 
> be coming in and deleting the connector while handle() is using it.
> [https://github.com/apache/qpid-dispatch/blob/main/src/server.c]
> {code:java}
>  989 /* Events involving a connection or listener are serialized by the 
> proactor so
>  990  * only one event per connection / listener will be processed at a time. 
>  
>  991  */  
>  992 static bool handle(qd_server_t *qd_server, pn_event_t *e, 
> pn_connection_t *pn_conn, qd_connection_t *ctx) 
>  993 {
>  
>  994 if (pn_conn && qdr_is_authentication_service_connection(pn_conn)) {  
>  
>  995 qdr_handle_authentication_service_connection_event(e);   
>  
>  996 return true; 
>  
>  997 }
>  
>  998  
>   
>  999 switch (pn_event_type(e)) { 
>  …
> 1051 case PN_TRANSPORT_ERROR: 
>  
> 1052 {
>  
> 1053 pn_transport_t *transport = pn_event_transport(e);   
>  
> 1054 pn_condition_t* condition = transport ? 
> pn_transport_condition(transport) : NULL; 
> 1055 if (ctx && ctx->connector) { /* Outgoing connection */   
>  
> 1056 qd_increment_conn_index(ctx);
>  
> 1057 const qd_server_config_t *config = 
> &ctx->connector->config;   
> 1058 ctx->connector->state = CXTR_STATE_FAILED;   
>  
> 1059 char conn_msg[300];  
>  
> 1060 if (condition  && pn_condition_is_set(condition)) {  
>  
> 1061 qd_format_string(conn_msg, 300, "[C%"PRIu64"] 
> Connection to %s failed: %s %s", ct

[jira] [Commented] (DISPATCH-2046) Panic in handle due to deleted connector.

2021-05-03 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/DISPATCH-2046?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17338437#comment-17338437
 ] 

ASF GitHub Bot commented on DISPATCH-2046:
--

codecov-commenter edited a comment on pull request #1176:
URL: https://github.com/apache/qpid-dispatch/pull/1176#issuecomment-829448935


   # 
[Codecov](https://codecov.io/gh/apache/qpid-dispatch/pull/1176?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
 Report
   > Merging 
[#1176](https://codecov.io/gh/apache/qpid-dispatch/pull/1176?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
 (7f06349) into 
[main](https://codecov.io/gh/apache/qpid-dispatch/commit/34b3c116dc97f9a1d7caf00fcea0e5e5144e6c45?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
 (34b3c11) will **decrease** coverage by `0.02%`.
   > The diff coverage is `89.16%`.
   
   [![Impacted file tree 
graph](https://codecov.io/gh/apache/qpid-dispatch/pull/1176/graphs/tree.svg?width=650&height=150&src=pr&token=rk2Cgd27pP&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/qpid-dispatch/pull/1176?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@Coverage Diff @@
   ## main#1176  +/-   ##
   ==
   - Coverage   84.32%   84.30%   -0.03% 
   ==
 Files 113  113  
 Lines   2797527995  +20 
   ==
   + Hits2359123602  +11 
   - Misses   4384 4393   +9 
   ```
   
   
   | [Impacted 
Files](https://codecov.io/gh/apache/qpid-dispatch/pull/1176?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
 | Coverage Δ | |
   |---|---|---|
   | 
[src/alloc\_pool.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1176/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL2FsbG9jX3Bvb2wuYw==)
 | `93.68% <ø> (ø)` | |
   | 
[src/connection\_manager.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1176/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL2Nvbm5lY3Rpb25fbWFuYWdlci5j)
 | `89.45% <88.88%> (-0.68%)` | :arrow_down: |
   | 
[src/server.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1176/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL3NlcnZlci5j)
 | `86.55% <89.01%> (-0.24%)` | :arrow_down: |
   | 
[src/router\_node.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1176/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL3JvdXRlcl9ub2RlLmM=)
 | `92.68% <100.00%> (+0.01%)` | :arrow_up: |
   | 
[src/router\_core/transfer.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1176/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL3JvdXRlcl9jb3JlL3RyYW5zZmVyLmM=)
 | `93.27% <0.00%> (-0.87%)` | :arrow_down: |
   | 
[src/router\_core/delivery.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1176/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL3JvdXRlcl9jb3JlL2RlbGl2ZXJ5LmM=)
 | `93.51% <0.00%> (-0.19%)` | :arrow_down: |
   | 
[src/iterator.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1176/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL2l0ZXJhdG9yLmM=)
 | `89.29% <0.00%> (-0.19%)` | :arrow_down: |
   | 
[src/router\_core/connections.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1176/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL3JvdXRlcl9jb3JlL2Nvbm5lY3Rpb25zLmM=)
 | `89.52% <0.00%> (+0.08%)` | :arrow_up: |
   | 
[src/adaptors/tcp\_adaptor.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1176/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+com

[jira] [Commented] (DISPATCH-2046) Panic in handle due to deleted connector.

2021-04-30 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/DISPATCH-2046?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17337519#comment-17337519
 ] 

ASF GitHub Bot commented on DISPATCH-2046:
--

ganeshmurthy closed pull request #1142:
URL: https://github.com/apache/qpid-dispatch/pull/1142


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


> Panic in handle due to deleted connector.
> -
>
> Key: DISPATCH-2046
> URL: https://issues.apache.org/jira/browse/DISPATCH-2046
> Project: Qpid Dispatch
>  Issue Type: Bug
>  Components: Routing Engine
>Affects Versions: 1.15.0
>Reporter: Alex Ward
>Assignee: Ken Giusti
>Priority: Blocker
> Fix For: 1.16.0
>
>
> I am seeing a segv at line 1063 due to ctx->connector being 0 in 
> server.c:handle(). 
> {code:java}
> (ugdb-x86_64-7.11-261) p ctx
> $6 = (qd_connection_t *) 0x802e55890
> (ugdb-x86_64-7.11-261) p *ctx
> $7 = {prev = 0x8023df490, next = 0x802e7a890, name = 0x0, server = 
> 0x80220d0c0, opened = false, closed = false, closed_locally = false, enqueued 
> = 0, timer = 0x0, pn_conn = 0x8023ec050, pn_sessions = {0x0  times>}, ssl = 0x0, listener = 0x0, connector = 0x0, connector_lock = 
> 0x8023e6600, context = 0x0, user_context = 0x0, link_context = 0x0, 
> connection_id = 12, user_id = 0x0, free_user_id = false, policy_settings = 
> 0x0, n_sessions = 0, n_senders = 0, n_receivers = 0, open_container = 0x0, 
> deferred_calls = {head = 0x0, tail = 0x0, scratch = 0x0, size = 0}, 
> deferred_call_lock = 0x8023e65c0, policy_counted = false, role = 0x801a17660 
> "route-container", free_link_session_list = {head = 0x0, tail = 0x0, scratch 
> = 0x0, size = 0}, strip_annotations_in = false, strip_annotations_out = 
> false, wake = 0x8005d6e20 , rhost = '\000'  times>, rhost_port = '\000' }
> {code}
> As the comment at the start of handle() states, there is only one event being 
> handled at a time, but it doesn’t look like there is anything to stop someone 
> clearing ctx->connector between lines 1055 and 1063. The python code seems to 
> be coming in and deleting the connector while handle() is using it.
> [https://github.com/apache/qpid-dispatch/blob/main/src/server.c]
> {code:java}
>  989 /* Events involving a connection or listener are serialized by the 
> proactor so
>  990  * only one event per connection / listener will be processed at a time. 
>  
>  991  */  
>  992 static bool handle(qd_server_t *qd_server, pn_event_t *e, 
> pn_connection_t *pn_conn, qd_connection_t *ctx) 
>  993 {
>  
>  994 if (pn_conn && qdr_is_authentication_service_connection(pn_conn)) {  
>  
>  995 qdr_handle_authentication_service_connection_event(e);   
>  
>  996 return true; 
>  
>  997 }
>  
>  998  
>   
>  999 switch (pn_event_type(e)) { 
>  …
> 1051 case PN_TRANSPORT_ERROR: 
>  
> 1052 {
>  
> 1053 pn_transport_t *transport = pn_event_transport(e);   
>  
> 1054 pn_condition_t* condition = transport ? 
> pn_transport_condition(transport) : NULL; 
> 1055 if (ctx && ctx->connector) { /* Outgoing connection */   
>  
> 1056 qd_increment_conn_index(ctx);
>  
> 1057 const qd_server_config_t *config = 
> &ctx->connector->config;   
> 1058 ctx->connector->state = CXTR_STATE_FAILED;   
>  
> 1059 char conn_msg[300];  
>  
> 1060 if (condition  && pn_condition_is_set(condition)) {  
>  
> 1061 qd_format_string(conn_msg, 300, "[C%"PRIu64"] 
> Connection to %s failed: %s %

[jira] [Commented] (DISPATCH-2046) Panic in handle due to deleted connector.

2021-04-30 Thread Ken Giusti (Jira)


[ 
https://issues.apache.org/jira/browse/DISPATCH-2046?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17337424#comment-17337424
 ] 

Ken Giusti commented on DISPATCH-2046:
--

Cannot reliably eliminate the timer reference count without ensuring the 
callback will not fire.

> Panic in handle due to deleted connector.
> -
>
> Key: DISPATCH-2046
> URL: https://issues.apache.org/jira/browse/DISPATCH-2046
> Project: Qpid Dispatch
>  Issue Type: Bug
>  Components: Routing Engine
>Affects Versions: 1.15.0
>Reporter: Alex Ward
>Assignee: Ken Giusti
>Priority: Blocker
> Fix For: 1.16.0
>
>
> I am seeing a segv at line 1063 due to ctx->connector being 0 in 
> server.c:handle(). 
> {code:java}
> (ugdb-x86_64-7.11-261) p ctx
> $6 = (qd_connection_t *) 0x802e55890
> (ugdb-x86_64-7.11-261) p *ctx
> $7 = {prev = 0x8023df490, next = 0x802e7a890, name = 0x0, server = 
> 0x80220d0c0, opened = false, closed = false, closed_locally = false, enqueued 
> = 0, timer = 0x0, pn_conn = 0x8023ec050, pn_sessions = {0x0  times>}, ssl = 0x0, listener = 0x0, connector = 0x0, connector_lock = 
> 0x8023e6600, context = 0x0, user_context = 0x0, link_context = 0x0, 
> connection_id = 12, user_id = 0x0, free_user_id = false, policy_settings = 
> 0x0, n_sessions = 0, n_senders = 0, n_receivers = 0, open_container = 0x0, 
> deferred_calls = {head = 0x0, tail = 0x0, scratch = 0x0, size = 0}, 
> deferred_call_lock = 0x8023e65c0, policy_counted = false, role = 0x801a17660 
> "route-container", free_link_session_list = {head = 0x0, tail = 0x0, scratch 
> = 0x0, size = 0}, strip_annotations_in = false, strip_annotations_out = 
> false, wake = 0x8005d6e20 , rhost = '\000'  times>, rhost_port = '\000' }
> {code}
> As the comment at the start of handle() states, there is only one event being 
> handled at a time, but it doesn’t look like there is anything to stop someone 
> clearing ctx->connector between lines 1055 and 1063. The python code seems to 
> be coming in and deleting the connector while handle() is using it.
> [https://github.com/apache/qpid-dispatch/blob/main/src/server.c]
> {code:java}
>  989 /* Events involving a connection or listener are serialized by the 
> proactor so
>  990  * only one event per connection / listener will be processed at a time. 
>  
>  991  */  
>  992 static bool handle(qd_server_t *qd_server, pn_event_t *e, 
> pn_connection_t *pn_conn, qd_connection_t *ctx) 
>  993 {
>  
>  994 if (pn_conn && qdr_is_authentication_service_connection(pn_conn)) {  
>  
>  995 qdr_handle_authentication_service_connection_event(e);   
>  
>  996 return true; 
>  
>  997 }
>  
>  998  
>   
>  999 switch (pn_event_type(e)) { 
>  …
> 1051 case PN_TRANSPORT_ERROR: 
>  
> 1052 {
>  
> 1053 pn_transport_t *transport = pn_event_transport(e);   
>  
> 1054 pn_condition_t* condition = transport ? 
> pn_transport_condition(transport) : NULL; 
> 1055 if (ctx && ctx->connector) { /* Outgoing connection */   
>  
> 1056 qd_increment_conn_index(ctx);
>  
> 1057 const qd_server_config_t *config = 
> &ctx->connector->config;   
> 1058 ctx->connector->state = CXTR_STATE_FAILED;   
>  
> 1059 char conn_msg[300];  
>  
> 1060 if (condition  && pn_condition_is_set(condition)) {  
>  
> 1061 qd_format_string(conn_msg, 300, "[C%"PRIu64"] 
> Connection to %s failed: %s %s", ctx->connection_id, config->host_port,   
> 
> 1062 pn_condition_get_name(condition), 
> pn_condition_get_description(condition));   
> 1063 strcp

[jira] [Commented] (DISPATCH-2046) Panic in handle due to deleted connector.

2021-04-29 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/DISPATCH-2046?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17337123#comment-17337123
 ] 

ASF GitHub Bot commented on DISPATCH-2046:
--

codecov-commenter edited a comment on pull request #1176:
URL: https://github.com/apache/qpid-dispatch/pull/1176#issuecomment-829448935


   # 
[Codecov](https://codecov.io/gh/apache/qpid-dispatch/pull/1176?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
 Report
   > Merging 
[#1176](https://codecov.io/gh/apache/qpid-dispatch/pull/1176?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
 (9915b44) into 
[main](https://codecov.io/gh/apache/qpid-dispatch/commit/a162fea6f602fe89e1e8d8eef96ca57a2adcb2a5?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
 (a162fea) will **decrease** coverage by `0.13%`.
   > The diff coverage is `89.16%`.
   
   [![Impacted file tree 
graph](https://codecov.io/gh/apache/qpid-dispatch/pull/1176/graphs/tree.svg?width=650&height=150&src=pr&token=rk2Cgd27pP&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/qpid-dispatch/pull/1176?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@Coverage Diff @@
   ## main#1176  +/-   ##
   ==
   - Coverage   84.42%   84.28%   -0.14% 
   ==
 Files 112  112  
 Lines   2769327714  +21 
   ==
   - Hits2338123360  -21 
   - Misses   4312 4354  +42 
   ```
   
   
   | [Impacted 
Files](https://codecov.io/gh/apache/qpid-dispatch/pull/1176?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
 | Coverage Δ | |
   |---|---|---|
   | 
[src/alloc\_pool.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1176/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL2FsbG9jX3Bvb2wuYw==)
 | `93.68% <ø> (ø)` | |
   | 
[src/connection\_manager.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1176/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL2Nvbm5lY3Rpb25fbWFuYWdlci5j)
 | `89.45% <88.88%> (-0.68%)` | :arrow_down: |
   | 
[src/server.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1176/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL3NlcnZlci5j)
 | `86.55% <89.01%> (-0.24%)` | :arrow_down: |
   | 
[src/router\_node.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1176/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL3JvdXRlcl9ub2RlLmM=)
 | `92.68% <100.00%> (-0.09%)` | :arrow_down: |
   | 
[...router\_core/modules/edge\_router/link\_route\_proxy.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1176/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL3JvdXRlcl9jb3JlL21vZHVsZXMvZWRnZV9yb3V0ZXIvbGlua19yb3V0ZV9wcm94eS5j)
 | `78.69% <0.00%> (-4.15%)` | :arrow_down: |
   | 
[src/iterator.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1176/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL2l0ZXJhdG9yLmM=)
 | `89.29% <0.00%> (-4.05%)` | :arrow_down: |
   | 
[src/router\_core/modules/edge\_router/edge\_mgmt.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1176/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL3JvdXRlcl9jb3JlL21vZHVsZXMvZWRnZV9yb3V0ZXIvZWRnZV9tZ210LmM=)
 | `84.15% <0.00%> (-1.00%)` | :arrow_down: |
   | 
[src/hash.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1176/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL2hhc2guYw==)
 | `79.53% <0.00%> (-0.47%)` | :arrow_down: |
   | 
[src/router\_core/connections.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1176/diff?src=pr&el=tr

[jira] [Commented] (DISPATCH-2046) Panic in handle due to deleted connector.

2021-04-29 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/DISPATCH-2046?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17335710#comment-17335710
 ] 

ASF GitHub Bot commented on DISPATCH-2046:
--

codecov-commenter commented on pull request #1176:
URL: https://github.com/apache/qpid-dispatch/pull/1176#issuecomment-829448935


   # 
[Codecov](https://codecov.io/gh/apache/qpid-dispatch/pull/1176?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
 Report
   > Merging 
[#1176](https://codecov.io/gh/apache/qpid-dispatch/pull/1176?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
 (11057d2) into 
[main](https://codecov.io/gh/apache/qpid-dispatch/commit/a162fea6f602fe89e1e8d8eef96ca57a2adcb2a5?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
 (a162fea) will **decrease** coverage by `0.16%`.
   > The diff coverage is `88.79%`.
   
   [![Impacted file tree 
graph](https://codecov.io/gh/apache/qpid-dispatch/pull/1176/graphs/tree.svg?width=650&height=150&src=pr&token=rk2Cgd27pP&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/qpid-dispatch/pull/1176?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@Coverage Diff @@
   ## main#1176  +/-   ##
   ==
   - Coverage   84.39%   84.23%   -0.17% 
   ==
 Files 112  112  
 Lines   2769327714  +21 
   ==
   - Hits2337223344  -28 
   - Misses   4321 4370  +49 
   ```
   
   
   | [Impacted 
Files](https://codecov.io/gh/apache/qpid-dispatch/pull/1176?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
 | Coverage Δ | |
   |---|---|---|
   | 
[src/alloc\_pool.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1176/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL2FsbG9jX3Bvb2wuYw==)
 | `93.68% <ø> (ø)` | |
   | 
[src/connection\_manager.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1176/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL2Nvbm5lY3Rpb25fbWFuYWdlci5j)
 | `89.45% <86.95%> (-0.68%)` | :arrow_down: |
   | 
[src/server.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1176/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL3NlcnZlci5j)
 | `86.55% <89.01%> (-0.24%)` | :arrow_down: |
   | 
[src/router\_node.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1176/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL3JvdXRlcl9ub2RlLmM=)
 | `92.68% <100.00%> (-0.09%)` | :arrow_down: |
   | 
[...router\_core/modules/edge\_router/link\_route\_proxy.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1176/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL3JvdXRlcl9jb3JlL21vZHVsZXMvZWRnZV9yb3V0ZXIvbGlua19yb3V0ZV9wcm94eS5j)
 | `78.69% <0.00%> (-4.15%)` | :arrow_down: |
   | 
[src/iterator.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1176/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL2l0ZXJhdG9yLmM=)
 | `89.46% <0.00%> (-3.89%)` | :arrow_down: |
   | 
[src/router\_core/modules/edge\_router/edge\_mgmt.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1176/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL3JvdXRlcl9jb3JlL21vZHVsZXMvZWRnZV9yb3V0ZXIvZWRnZV9tZ210LmM=)
 | `84.15% <0.00%> (-1.00%)` | :arrow_down: |
   | 
[src/router\_core/router\_core.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1176/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL3JvdXRlcl9jb3JlL3JvdXRlcl9jb3JlLmM=)
 | `86.26% <0.00%> (-0.97%)` | :arrow_down: |
   | 
[src/router\_core/transfer.c](https://codecov.io/gh/apache/qpid-d

[jira] [Commented] (DISPATCH-2046) Panic in handle due to deleted connector.

2021-04-29 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/DISPATCH-2046?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17335682#comment-17335682
 ] 

ASF GitHub Bot commented on DISPATCH-2046:
--

kgiusti commented on a change in pull request #1176:
URL: https://github.com/apache/qpid-dispatch/pull/1176#discussion_r623201753



##
File path: src/server.c
##
@@ -1644,25 +1657,31 @@ void qd_listener_decref(qd_listener_t* li)
 
 qd_connector_t *qd_server_connector(qd_server_t *server)
 {
-qd_connector_t *ct = new_qd_connector_t();
-if (!ct) return 0;
-ZERO(ct);
-sys_atomic_init(&ct->ref_count, 1);
-ct->server  = server;
-qd_failover_item_list_t conn_info_list;
-DEQ_INIT(conn_info_list);
-ct->conn_info_list = conn_info_list;
-ct->conn_index = 1;
-ct->state   = CXTR_STATE_INIT;
-ct->lock = sys_mutex();
-ct->conn_msg = (char*) malloc(300);
-memset(ct->conn_msg, 0, 300);
-ct->timer = qd_timer(ct->server->qd, try_open_cb, ct);
-if (!ct->lock || !ct->timer) {
-qd_connector_decref(ct);
-return 0;
-}
-return ct;
+qd_connector_t *connector = new_qd_connector_t();
+if (!connector) return 0;
+ZERO(connector);
+sys_atomic_init(&connector->ref_count, 1);
+DEQ_INIT(connector->conn_info_list);
+
+connector->server  = server;

Review comment:
   How does that make a difference?  I don't understand - the other stuff 
(conn_index, server, etc) are idempotent - it doesn't matter if they are set or 
not if the initialization fails.
   
   If it's a matter of preference then no problem - I just don't see any 
advantage to that change.

##
File path: src/server.c
##
@@ -1644,25 +1657,31 @@ void qd_listener_decref(qd_listener_t* li)
 
 qd_connector_t *qd_server_connector(qd_server_t *server)
 {
-qd_connector_t *ct = new_qd_connector_t();
-if (!ct) return 0;
-ZERO(ct);
-sys_atomic_init(&ct->ref_count, 1);
-ct->server  = server;
-qd_failover_item_list_t conn_info_list;
-DEQ_INIT(conn_info_list);
-ct->conn_info_list = conn_info_list;
-ct->conn_index = 1;
-ct->state   = CXTR_STATE_INIT;
-ct->lock = sys_mutex();
-ct->conn_msg = (char*) malloc(300);
-memset(ct->conn_msg, 0, 300);
-ct->timer = qd_timer(ct->server->qd, try_open_cb, ct);
-if (!ct->lock || !ct->timer) {
-qd_connector_decref(ct);
-return 0;
-}
-return ct;
+qd_connector_t *connector = new_qd_connector_t();
+if (!connector) return 0;
+ZERO(connector);
+sys_atomic_init(&connector->ref_count, 1);
+DEQ_INIT(connector->conn_info_list);
+
+connector->server  = server;
+connector->conn_index = 1;
+connector->state = CXTR_STATE_INIT;
+connector->lock = sys_mutex();
+if (!connector->lock)
+goto error;
+connector->timer = qd_timer(connector->server->qd, try_open_cb, connector);
+if (!connector->timer)
+goto error;
+connector->conn_msg = (char*) malloc(300);
+if (!connector->conn_msg)
+goto error;
+memset(connector->conn_msg, 0, 300);
+return connector;
+
+error:

Review comment:
   Triggered!
   
   goto's for error handling in C code are not evil.  In fact, when using an 
RAII resource allocation model they simplify the code.  For example, if I had 
to check every allocation and clean up on each check, you'd get:
   
   `allocate_lock();`
   `if (connector->lock) {`
   `   allocate_timer();`
   `   if (timer) {`
   `  allocate conn_msg;`
   `  if (conn_msg) {`
   `  memset conn_msg;`
   `  return connector`
   `  } else {`
   `   free_timer();`
   `   free_lock();`
   `   } else {`
   `  free_lock();`
   `   }`
   `}`
   `decref();`
   `return 0;`
   
   
   Is that code actually better than the goto version?Now assume you make a 
change where you need to allocate something else.  In the if case you'll have 
to add another level of indentation and update each separate cleanup "else" to 
deal with unwinding the new resource.
   
   Google "arrow anti-pattern" for another example.

##
File path: src/connection_manager.c
##
@@ -790,6 +790,8 @@ qd_error_t qd_entity_refresh_connector(qd_entity_t* entity, 
void *impl)
 int i = 1;
 int num_items = 0;
 
+sys_mutex_lock(ct->lock);
+

Review comment:
   Remember the connector will be in the "deleted" state until the related 
connection has closed. 
   Would it be better to report the state as "CLOSING" instead of "DELETED"?  
At least if - for whatever reason - the connection hasn't closed we don't 
ignore connectors that still exist.

##
File path: src/server.c
##
@@ -1155,56 +1156,55 @@ static qd_failover_item_t 
*qd_connector_get_conn_info(qd_connector_t *ct) {
 
 
 /* Timer callback to try/retry connection open */
-static void try_open_lh(qd_connector_t *ct)
+st

[jira] [Commented] (DISPATCH-2046) Panic in handle due to deleted connector.

2021-04-29 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/DISPATCH-2046?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17335552#comment-17335552
 ] 

ASF GitHub Bot commented on DISPATCH-2046:
--

ganeshmurthy commented on a change in pull request #1176:
URL: https://github.com/apache/qpid-dispatch/pull/1176#discussion_r623130386



##
File path: src/connection_manager.c
##
@@ -790,6 +790,8 @@ qd_error_t qd_entity_refresh_connector(qd_entity_t* entity, 
void *impl)
 int i = 1;
 int num_items = 0;
 
+sys_mutex_lock(ct->lock);
+

Review comment:
   If the connector state is CXTR_STATE_DELETED, should we just return 
immediately? Why loop thru the conn_info_list and so on? When the user runs a 
qdmanage QUERY --type=connector, I think the deleted connector 
(CXTR_STATE_DELETED) should not even show up in the list of connectors.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


> Panic in handle due to deleted connector.
> -
>
> Key: DISPATCH-2046
> URL: https://issues.apache.org/jira/browse/DISPATCH-2046
> Project: Qpid Dispatch
>  Issue Type: Bug
>  Components: Routing Engine
>Affects Versions: 1.15.0
>Reporter: Alex Ward
>Assignee: Ken Giusti
>Priority: Major
> Fix For: 1.17.0
>
>
> I am seeing a segv at line 1063 due to ctx->connector being 0 in 
> server.c:handle(). 
> {code:java}
> (ugdb-x86_64-7.11-261) p ctx
> $6 = (qd_connection_t *) 0x802e55890
> (ugdb-x86_64-7.11-261) p *ctx
> $7 = {prev = 0x8023df490, next = 0x802e7a890, name = 0x0, server = 
> 0x80220d0c0, opened = false, closed = false, closed_locally = false, enqueued 
> = 0, timer = 0x0, pn_conn = 0x8023ec050, pn_sessions = {0x0  times>}, ssl = 0x0, listener = 0x0, connector = 0x0, connector_lock = 
> 0x8023e6600, context = 0x0, user_context = 0x0, link_context = 0x0, 
> connection_id = 12, user_id = 0x0, free_user_id = false, policy_settings = 
> 0x0, n_sessions = 0, n_senders = 0, n_receivers = 0, open_container = 0x0, 
> deferred_calls = {head = 0x0, tail = 0x0, scratch = 0x0, size = 0}, 
> deferred_call_lock = 0x8023e65c0, policy_counted = false, role = 0x801a17660 
> "route-container", free_link_session_list = {head = 0x0, tail = 0x0, scratch 
> = 0x0, size = 0}, strip_annotations_in = false, strip_annotations_out = 
> false, wake = 0x8005d6e20 , rhost = '\000'  times>, rhost_port = '\000' }
> {code}
> As the comment at the start of handle() states, there is only one event being 
> handled at a time, but it doesn’t look like there is anything to stop someone 
> clearing ctx->connector between lines 1055 and 1063. The python code seems to 
> be coming in and deleting the connector while handle() is using it.
> [https://github.com/apache/qpid-dispatch/blob/main/src/server.c]
> {code:java}
>  989 /* Events involving a connection or listener are serialized by the 
> proactor so
>  990  * only one event per connection / listener will be processed at a time. 
>  
>  991  */  
>  992 static bool handle(qd_server_t *qd_server, pn_event_t *e, 
> pn_connection_t *pn_conn, qd_connection_t *ctx) 
>  993 {
>  
>  994 if (pn_conn && qdr_is_authentication_service_connection(pn_conn)) {  
>  
>  995 qdr_handle_authentication_service_connection_event(e);   
>  
>  996 return true; 
>  
>  997 }
>  
>  998  
>   
>  999 switch (pn_event_type(e)) { 
>  …
> 1051 case PN_TRANSPORT_ERROR: 
>  
> 1052 {
>  
> 1053 pn_transport_t *transport = pn_event_transport(e);   
>  
> 1054 pn_condition_t* condition = transport ? 
> pn_transport_condition(transport) : NULL; 
> 1055 if (ctx && ctx->connector) { /* Outgoing connection */   
>  
> 1056 qd_increment_conn_index(ctx);
>  
> 

[jira] [Commented] (DISPATCH-2046) Panic in handle due to deleted connector.

2021-04-29 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/DISPATCH-2046?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17335541#comment-17335541
 ] 

ASF GitHub Bot commented on DISPATCH-2046:
--

ganeshmurthy commented on a change in pull request #1176:
URL: https://github.com/apache/qpid-dispatch/pull/1176#discussion_r623117544



##
File path: src/alloc_pool.c
##
@@ -90,7 +90,7 @@ DEQ_DECLARE(qd_alloc_type_t, qd_alloc_type_list_t);
 #if QD_MEMORY_STATS
 static const char *leaking_types[] = {
 // DISPATCH-1679:
-"qd_timer_t", "qd_connector_t",
+//"qd_timer_t", "qd_connector_t",

Review comment:
   Just delete the two lines. We could add a link in the JIRA DISPATCH-2046 
about DISPATCH-1679




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


> Panic in handle due to deleted connector.
> -
>
> Key: DISPATCH-2046
> URL: https://issues.apache.org/jira/browse/DISPATCH-2046
> Project: Qpid Dispatch
>  Issue Type: Bug
>  Components: Routing Engine
>Affects Versions: 1.15.0
>Reporter: Alex Ward
>Assignee: Ken Giusti
>Priority: Major
> Fix For: 1.17.0
>
>
> I am seeing a segv at line 1063 due to ctx->connector being 0 in 
> server.c:handle(). 
> {code:java}
> (ugdb-x86_64-7.11-261) p ctx
> $6 = (qd_connection_t *) 0x802e55890
> (ugdb-x86_64-7.11-261) p *ctx
> $7 = {prev = 0x8023df490, next = 0x802e7a890, name = 0x0, server = 
> 0x80220d0c0, opened = false, closed = false, closed_locally = false, enqueued 
> = 0, timer = 0x0, pn_conn = 0x8023ec050, pn_sessions = {0x0  times>}, ssl = 0x0, listener = 0x0, connector = 0x0, connector_lock = 
> 0x8023e6600, context = 0x0, user_context = 0x0, link_context = 0x0, 
> connection_id = 12, user_id = 0x0, free_user_id = false, policy_settings = 
> 0x0, n_sessions = 0, n_senders = 0, n_receivers = 0, open_container = 0x0, 
> deferred_calls = {head = 0x0, tail = 0x0, scratch = 0x0, size = 0}, 
> deferred_call_lock = 0x8023e65c0, policy_counted = false, role = 0x801a17660 
> "route-container", free_link_session_list = {head = 0x0, tail = 0x0, scratch 
> = 0x0, size = 0}, strip_annotations_in = false, strip_annotations_out = 
> false, wake = 0x8005d6e20 , rhost = '\000'  times>, rhost_port = '\000' }
> {code}
> As the comment at the start of handle() states, there is only one event being 
> handled at a time, but it doesn’t look like there is anything to stop someone 
> clearing ctx->connector between lines 1055 and 1063. The python code seems to 
> be coming in and deleting the connector while handle() is using it.
> [https://github.com/apache/qpid-dispatch/blob/main/src/server.c]
> {code:java}
>  989 /* Events involving a connection or listener are serialized by the 
> proactor so
>  990  * only one event per connection / listener will be processed at a time. 
>  
>  991  */  
>  992 static bool handle(qd_server_t *qd_server, pn_event_t *e, 
> pn_connection_t *pn_conn, qd_connection_t *ctx) 
>  993 {
>  
>  994 if (pn_conn && qdr_is_authentication_service_connection(pn_conn)) {  
>  
>  995 qdr_handle_authentication_service_connection_event(e);   
>  
>  996 return true; 
>  
>  997 }
>  
>  998  
>   
>  999 switch (pn_event_type(e)) { 
>  …
> 1051 case PN_TRANSPORT_ERROR: 
>  
> 1052 {
>  
> 1053 pn_transport_t *transport = pn_event_transport(e);   
>  
> 1054 pn_condition_t* condition = transport ? 
> pn_transport_condition(transport) : NULL; 
> 1055 if (ctx && ctx->connector) { /* Outgoing connection */   
>  
> 1056 qd_increment_conn_index(ctx);
>  
> 1057 const qd_server_config_t *config = 
> &ctx->connector->config;   
> 1058   

[jira] [Commented] (DISPATCH-2046) Panic in handle due to deleted connector.

2021-04-29 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/DISPATCH-2046?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17335537#comment-17335537
 ] 

ASF GitHub Bot commented on DISPATCH-2046:
--

ganeshmurthy commented on a change in pull request #1176:
URL: https://github.com/apache/qpid-dispatch/pull/1176#discussion_r623113540



##
File path: src/server.c
##
@@ -1155,56 +1156,55 @@ static qd_failover_item_t 
*qd_connector_get_conn_info(qd_connector_t *ct) {
 
 
 /* Timer callback to try/retry connection open */
-static void try_open_lh(qd_connector_t *ct)
+static void try_open_lh(qd_connector_t *connector)
 {
-if (ct->state != CXTR_STATE_CONNECTING && ct->state != CXTR_STATE_INIT) {
-/* No longer referenced by pn_connection or timer */
-qd_connector_decref(ct);
+// Allocate a new connection. No other thread will touch this
+// connection until pn_proactor_connect is called below
+qd_connection_t *qd_conn = qd_server_connection(connector->server, 
&connector->config);
+if (!qd_conn) { /* Try again later */
+qd_log(connector->server->log_source, QD_LOG_CRITICAL, "Allocation 
failure connecting to %s",
+   connector->config.host_port);
+connector->delay = 1;
+connector->state = CXTR_STATE_CONNECTING;

Review comment:
   Logically speaking, should the state here be CXTR_STATE_FAILED ? After 
all the qd_server_connection() failed to allocate a new qd_connection_t object. 
When the next timer fires after 10 seconds, it will set the state to 
CXTR_STATE_CONNECTING, right? 




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


> Panic in handle due to deleted connector.
> -
>
> Key: DISPATCH-2046
> URL: https://issues.apache.org/jira/browse/DISPATCH-2046
> Project: Qpid Dispatch
>  Issue Type: Bug
>  Components: Routing Engine
>Affects Versions: 1.15.0
>Reporter: Alex Ward
>Assignee: Ken Giusti
>Priority: Major
> Fix For: 1.17.0
>
>
> I am seeing a segv at line 1063 due to ctx->connector being 0 in 
> server.c:handle(). 
> {code:java}
> (ugdb-x86_64-7.11-261) p ctx
> $6 = (qd_connection_t *) 0x802e55890
> (ugdb-x86_64-7.11-261) p *ctx
> $7 = {prev = 0x8023df490, next = 0x802e7a890, name = 0x0, server = 
> 0x80220d0c0, opened = false, closed = false, closed_locally = false, enqueued 
> = 0, timer = 0x0, pn_conn = 0x8023ec050, pn_sessions = {0x0  times>}, ssl = 0x0, listener = 0x0, connector = 0x0, connector_lock = 
> 0x8023e6600, context = 0x0, user_context = 0x0, link_context = 0x0, 
> connection_id = 12, user_id = 0x0, free_user_id = false, policy_settings = 
> 0x0, n_sessions = 0, n_senders = 0, n_receivers = 0, open_container = 0x0, 
> deferred_calls = {head = 0x0, tail = 0x0, scratch = 0x0, size = 0}, 
> deferred_call_lock = 0x8023e65c0, policy_counted = false, role = 0x801a17660 
> "route-container", free_link_session_list = {head = 0x0, tail = 0x0, scratch 
> = 0x0, size = 0}, strip_annotations_in = false, strip_annotations_out = 
> false, wake = 0x8005d6e20 , rhost = '\000'  times>, rhost_port = '\000' }
> {code}
> As the comment at the start of handle() states, there is only one event being 
> handled at a time, but it doesn’t look like there is anything to stop someone 
> clearing ctx->connector between lines 1055 and 1063. The python code seems to 
> be coming in and deleting the connector while handle() is using it.
> [https://github.com/apache/qpid-dispatch/blob/main/src/server.c]
> {code:java}
>  989 /* Events involving a connection or listener are serialized by the 
> proactor so
>  990  * only one event per connection / listener will be processed at a time. 
>  
>  991  */  
>  992 static bool handle(qd_server_t *qd_server, pn_event_t *e, 
> pn_connection_t *pn_conn, qd_connection_t *ctx) 
>  993 {
>  
>  994 if (pn_conn && qdr_is_authentication_service_connection(pn_conn)) {  
>  
>  995 qdr_handle_authentication_service_connection_event(e);   
>  
>  996 return true; 
>  
>  997 }
>  
>  998  
>   
>  999 switch (

[jira] [Commented] (DISPATCH-2046) Panic in handle due to deleted connector.

2021-04-29 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/DISPATCH-2046?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17335521#comment-17335521
 ] 

ASF GitHub Bot commented on DISPATCH-2046:
--

ganeshmurthy commented on a change in pull request #1176:
URL: https://github.com/apache/qpid-dispatch/pull/1176#discussion_r623093115



##
File path: src/server.c
##
@@ -1644,25 +1657,31 @@ void qd_listener_decref(qd_listener_t* li)
 
 qd_connector_t *qd_server_connector(qd_server_t *server)
 {
-qd_connector_t *ct = new_qd_connector_t();
-if (!ct) return 0;
-ZERO(ct);
-sys_atomic_init(&ct->ref_count, 1);
-ct->server  = server;
-qd_failover_item_list_t conn_info_list;
-DEQ_INIT(conn_info_list);
-ct->conn_info_list = conn_info_list;
-ct->conn_index = 1;
-ct->state   = CXTR_STATE_INIT;
-ct->lock = sys_mutex();
-ct->conn_msg = (char*) malloc(300);
-memset(ct->conn_msg, 0, 300);
-ct->timer = qd_timer(ct->server->qd, try_open_cb, ct);
-if (!ct->lock || !ct->timer) {
-qd_connector_decref(ct);
-return 0;
-}
-return ct;
+qd_connector_t *connector = new_qd_connector_t();
+if (!connector) return 0;
+ZERO(connector);
+sys_atomic_init(&connector->ref_count, 1);
+DEQ_INIT(connector->conn_info_list);
+
+connector->server  = server;
+connector->conn_index = 1;
+connector->state = CXTR_STATE_INIT;
+connector->lock = sys_mutex();
+if (!connector->lock)
+goto error;
+connector->timer = qd_timer(connector->server->qd, try_open_cb, connector);
+if (!connector->timer)
+goto error;
+connector->conn_msg = (char*) malloc(300);
+if (!connector->conn_msg)
+goto error;
+memset(connector->conn_msg, 0, 300);
+return connector;
+
+error:

Review comment:
   Can we please remove "goto" if you don't feel strongly about it?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


> Panic in handle due to deleted connector.
> -
>
> Key: DISPATCH-2046
> URL: https://issues.apache.org/jira/browse/DISPATCH-2046
> Project: Qpid Dispatch
>  Issue Type: Bug
>  Components: Routing Engine
>Affects Versions: 1.15.0
>Reporter: Alex Ward
>Assignee: Ken Giusti
>Priority: Major
> Fix For: 1.17.0
>
>
> I am seeing a segv at line 1063 due to ctx->connector being 0 in 
> server.c:handle(). 
> {code:java}
> (ugdb-x86_64-7.11-261) p ctx
> $6 = (qd_connection_t *) 0x802e55890
> (ugdb-x86_64-7.11-261) p *ctx
> $7 = {prev = 0x8023df490, next = 0x802e7a890, name = 0x0, server = 
> 0x80220d0c0, opened = false, closed = false, closed_locally = false, enqueued 
> = 0, timer = 0x0, pn_conn = 0x8023ec050, pn_sessions = {0x0  times>}, ssl = 0x0, listener = 0x0, connector = 0x0, connector_lock = 
> 0x8023e6600, context = 0x0, user_context = 0x0, link_context = 0x0, 
> connection_id = 12, user_id = 0x0, free_user_id = false, policy_settings = 
> 0x0, n_sessions = 0, n_senders = 0, n_receivers = 0, open_container = 0x0, 
> deferred_calls = {head = 0x0, tail = 0x0, scratch = 0x0, size = 0}, 
> deferred_call_lock = 0x8023e65c0, policy_counted = false, role = 0x801a17660 
> "route-container", free_link_session_list = {head = 0x0, tail = 0x0, scratch 
> = 0x0, size = 0}, strip_annotations_in = false, strip_annotations_out = 
> false, wake = 0x8005d6e20 , rhost = '\000'  times>, rhost_port = '\000' }
> {code}
> As the comment at the start of handle() states, there is only one event being 
> handled at a time, but it doesn’t look like there is anything to stop someone 
> clearing ctx->connector between lines 1055 and 1063. The python code seems to 
> be coming in and deleting the connector while handle() is using it.
> [https://github.com/apache/qpid-dispatch/blob/main/src/server.c]
> {code:java}
>  989 /* Events involving a connection or listener are serialized by the 
> proactor so
>  990  * only one event per connection / listener will be processed at a time. 
>  
>  991  */  
>  992 static bool handle(qd_server_t *qd_server, pn_event_t *e, 
> pn_connection_t *pn_conn, qd_connection_t *ctx) 
>  993 {
>  
>  994 if (pn_conn && qdr_is_authentication_service_connection(pn_conn)) {  
>  
>  995 qdr_handle_authentication_service_connection_event(e);   
>  
>  996 return true;

[jira] [Commented] (DISPATCH-2046) Panic in handle due to deleted connector.

2021-04-29 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/DISPATCH-2046?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17335519#comment-17335519
 ] 

ASF GitHub Bot commented on DISPATCH-2046:
--

ganeshmurthy commented on a change in pull request #1176:
URL: https://github.com/apache/qpid-dispatch/pull/1176#discussion_r623091408



##
File path: src/connection_manager.c
##
@@ -859,16 +861,23 @@ qd_error_t qd_entity_refresh_connector(qd_entity_t* 
entity, void *impl)
 case CXTR_STATE_INIT:
   state_info = "INITIALIZING";
   break;
+case CXTR_STATE_DELETED:
+  state_info = "DELETED";
+  break;
 default:
   state_info = "UNKNOWN";
   break;
 }
 
 if (qd_entity_set_string(entity, "failoverUrls", failover_info) == 0
-&& qd_entity_set_string(entity, "connectionStatus", state_info) == 0
-&& qd_entity_set_string(entity, "connectionMsg", ct->conn_msg) == 
0)
+&& qd_entity_set_string(entity, "connectionStatus", state_info) == 0
+&& qd_entity_set_string(entity, "connectionMsg", ct->conn_msg) == 0) {
+
+sys_mutex_unlock(ct->lock);

Review comment:
   can we change this to connector->lock ? In some places we have ct->lock 
and in some places we have connector->lock. The connector->lock is easier to 
understand IMO.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


> Panic in handle due to deleted connector.
> -
>
> Key: DISPATCH-2046
> URL: https://issues.apache.org/jira/browse/DISPATCH-2046
> Project: Qpid Dispatch
>  Issue Type: Bug
>  Components: Routing Engine
>Affects Versions: 1.15.0
>Reporter: Alex Ward
>Assignee: Ken Giusti
>Priority: Major
> Fix For: 1.17.0
>
>
> I am seeing a segv at line 1063 due to ctx->connector being 0 in 
> server.c:handle(). 
> {code:java}
> (ugdb-x86_64-7.11-261) p ctx
> $6 = (qd_connection_t *) 0x802e55890
> (ugdb-x86_64-7.11-261) p *ctx
> $7 = {prev = 0x8023df490, next = 0x802e7a890, name = 0x0, server = 
> 0x80220d0c0, opened = false, closed = false, closed_locally = false, enqueued 
> = 0, timer = 0x0, pn_conn = 0x8023ec050, pn_sessions = {0x0  times>}, ssl = 0x0, listener = 0x0, connector = 0x0, connector_lock = 
> 0x8023e6600, context = 0x0, user_context = 0x0, link_context = 0x0, 
> connection_id = 12, user_id = 0x0, free_user_id = false, policy_settings = 
> 0x0, n_sessions = 0, n_senders = 0, n_receivers = 0, open_container = 0x0, 
> deferred_calls = {head = 0x0, tail = 0x0, scratch = 0x0, size = 0}, 
> deferred_call_lock = 0x8023e65c0, policy_counted = false, role = 0x801a17660 
> "route-container", free_link_session_list = {head = 0x0, tail = 0x0, scratch 
> = 0x0, size = 0}, strip_annotations_in = false, strip_annotations_out = 
> false, wake = 0x8005d6e20 , rhost = '\000'  times>, rhost_port = '\000' }
> {code}
> As the comment at the start of handle() states, there is only one event being 
> handled at a time, but it doesn’t look like there is anything to stop someone 
> clearing ctx->connector between lines 1055 and 1063. The python code seems to 
> be coming in and deleting the connector while handle() is using it.
> [https://github.com/apache/qpid-dispatch/blob/main/src/server.c]
> {code:java}
>  989 /* Events involving a connection or listener are serialized by the 
> proactor so
>  990  * only one event per connection / listener will be processed at a time. 
>  
>  991  */  
>  992 static bool handle(qd_server_t *qd_server, pn_event_t *e, 
> pn_connection_t *pn_conn, qd_connection_t *ctx) 
>  993 {
>  
>  994 if (pn_conn && qdr_is_authentication_service_connection(pn_conn)) {  
>  
>  995 qdr_handle_authentication_service_connection_event(e);   
>  
>  996 return true; 
>  
>  997 }
>  
>  998  
>   
>  999 switch (pn_event_type(e)) { 
>  …
> 1051 case PN_TRANSPORT_ERROR: 
>  
> 1052 {
>  
> 10

[jira] [Commented] (DISPATCH-2046) Panic in handle due to deleted connector.

2021-04-29 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/DISPATCH-2046?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17335515#comment-17335515
 ] 

ASF GitHub Bot commented on DISPATCH-2046:
--

ganeshmurthy commented on a change in pull request #1176:
URL: https://github.com/apache/qpid-dispatch/pull/1176#discussion_r623086323



##
File path: src/server.c
##
@@ -1644,25 +1657,31 @@ void qd_listener_decref(qd_listener_t* li)
 
 qd_connector_t *qd_server_connector(qd_server_t *server)
 {
-qd_connector_t *ct = new_qd_connector_t();
-if (!ct) return 0;
-ZERO(ct);
-sys_atomic_init(&ct->ref_count, 1);
-ct->server  = server;
-qd_failover_item_list_t conn_info_list;
-DEQ_INIT(conn_info_list);
-ct->conn_info_list = conn_info_list;
-ct->conn_index = 1;
-ct->state   = CXTR_STATE_INIT;
-ct->lock = sys_mutex();
-ct->conn_msg = (char*) malloc(300);
-memset(ct->conn_msg, 0, 300);
-ct->timer = qd_timer(ct->server->qd, try_open_cb, ct);
-if (!ct->lock || !ct->timer) {
-qd_connector_decref(ct);
-return 0;
-}
-return ct;
+qd_connector_t *connector = new_qd_connector_t();
+if (!connector) return 0;
+ZERO(connector);
+sys_atomic_init(&connector->ref_count, 1);
+DEQ_INIT(connector->conn_info_list);
+
+connector->server  = server;

Review comment:
   How about initializing the timer, conn_msg, lock *first* and if those 
are successful, then set server, conn_index, etc.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


> Panic in handle due to deleted connector.
> -
>
> Key: DISPATCH-2046
> URL: https://issues.apache.org/jira/browse/DISPATCH-2046
> Project: Qpid Dispatch
>  Issue Type: Bug
>  Components: Routing Engine
>Affects Versions: 1.15.0
>Reporter: Alex Ward
>Assignee: Ken Giusti
>Priority: Major
> Fix For: 1.17.0
>
>
> I am seeing a segv at line 1063 due to ctx->connector being 0 in 
> server.c:handle(). 
> {code:java}
> (ugdb-x86_64-7.11-261) p ctx
> $6 = (qd_connection_t *) 0x802e55890
> (ugdb-x86_64-7.11-261) p *ctx
> $7 = {prev = 0x8023df490, next = 0x802e7a890, name = 0x0, server = 
> 0x80220d0c0, opened = false, closed = false, closed_locally = false, enqueued 
> = 0, timer = 0x0, pn_conn = 0x8023ec050, pn_sessions = {0x0  times>}, ssl = 0x0, listener = 0x0, connector = 0x0, connector_lock = 
> 0x8023e6600, context = 0x0, user_context = 0x0, link_context = 0x0, 
> connection_id = 12, user_id = 0x0, free_user_id = false, policy_settings = 
> 0x0, n_sessions = 0, n_senders = 0, n_receivers = 0, open_container = 0x0, 
> deferred_calls = {head = 0x0, tail = 0x0, scratch = 0x0, size = 0}, 
> deferred_call_lock = 0x8023e65c0, policy_counted = false, role = 0x801a17660 
> "route-container", free_link_session_list = {head = 0x0, tail = 0x0, scratch 
> = 0x0, size = 0}, strip_annotations_in = false, strip_annotations_out = 
> false, wake = 0x8005d6e20 , rhost = '\000'  times>, rhost_port = '\000' }
> {code}
> As the comment at the start of handle() states, there is only one event being 
> handled at a time, but it doesn’t look like there is anything to stop someone 
> clearing ctx->connector between lines 1055 and 1063. The python code seems to 
> be coming in and deleting the connector while handle() is using it.
> [https://github.com/apache/qpid-dispatch/blob/main/src/server.c]
> {code:java}
>  989 /* Events involving a connection or listener are serialized by the 
> proactor so
>  990  * only one event per connection / listener will be processed at a time. 
>  
>  991  */  
>  992 static bool handle(qd_server_t *qd_server, pn_event_t *e, 
> pn_connection_t *pn_conn, qd_connection_t *ctx) 
>  993 {
>  
>  994 if (pn_conn && qdr_is_authentication_service_connection(pn_conn)) {  
>  
>  995 qdr_handle_authentication_service_connection_event(e);   
>  
>  996 return true; 
>  
>  997 }
>  
>  998  
>   
>  999 switch (pn_event_type(e)) { 
>  …
> 1051 case PN_TRANSPORT_ERROR: 

[jira] [Commented] (DISPATCH-2046) Panic in handle due to deleted connector.

2021-04-29 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/DISPATCH-2046?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17335502#comment-17335502
 ] 

ASF GitHub Bot commented on DISPATCH-2046:
--

kgiusti opened a new pull request #1176:
URL: https://github.com/apache/qpid-dispatch/pull/1176


   DISPATCH-1679: fix qd_connector_t leak
   DISPATCH-1917: fix race when accessing connector->conn_msg buffer


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


> Panic in handle due to deleted connector.
> -
>
> Key: DISPATCH-2046
> URL: https://issues.apache.org/jira/browse/DISPATCH-2046
> Project: Qpid Dispatch
>  Issue Type: Bug
>  Components: Routing Engine
>Affects Versions: 1.15.0
>Reporter: Alex Ward
>Assignee: Ken Giusti
>Priority: Major
> Fix For: 1.17.0
>
>
> I am seeing a segv at line 1063 due to ctx->connector being 0 in 
> server.c:handle(). 
> {code:java}
> (ugdb-x86_64-7.11-261) p ctx
> $6 = (qd_connection_t *) 0x802e55890
> (ugdb-x86_64-7.11-261) p *ctx
> $7 = {prev = 0x8023df490, next = 0x802e7a890, name = 0x0, server = 
> 0x80220d0c0, opened = false, closed = false, closed_locally = false, enqueued 
> = 0, timer = 0x0, pn_conn = 0x8023ec050, pn_sessions = {0x0  times>}, ssl = 0x0, listener = 0x0, connector = 0x0, connector_lock = 
> 0x8023e6600, context = 0x0, user_context = 0x0, link_context = 0x0, 
> connection_id = 12, user_id = 0x0, free_user_id = false, policy_settings = 
> 0x0, n_sessions = 0, n_senders = 0, n_receivers = 0, open_container = 0x0, 
> deferred_calls = {head = 0x0, tail = 0x0, scratch = 0x0, size = 0}, 
> deferred_call_lock = 0x8023e65c0, policy_counted = false, role = 0x801a17660 
> "route-container", free_link_session_list = {head = 0x0, tail = 0x0, scratch 
> = 0x0, size = 0}, strip_annotations_in = false, strip_annotations_out = 
> false, wake = 0x8005d6e20 , rhost = '\000'  times>, rhost_port = '\000' }
> {code}
> As the comment at the start of handle() states, there is only one event being 
> handled at a time, but it doesn’t look like there is anything to stop someone 
> clearing ctx->connector between lines 1055 and 1063. The python code seems to 
> be coming in and deleting the connector while handle() is using it.
> [https://github.com/apache/qpid-dispatch/blob/main/src/server.c]
> {code:java}
>  989 /* Events involving a connection or listener are serialized by the 
> proactor so
>  990  * only one event per connection / listener will be processed at a time. 
>  
>  991  */  
>  992 static bool handle(qd_server_t *qd_server, pn_event_t *e, 
> pn_connection_t *pn_conn, qd_connection_t *ctx) 
>  993 {
>  
>  994 if (pn_conn && qdr_is_authentication_service_connection(pn_conn)) {  
>  
>  995 qdr_handle_authentication_service_connection_event(e);   
>  
>  996 return true; 
>  
>  997 }
>  
>  998  
>   
>  999 switch (pn_event_type(e)) { 
>  …
> 1051 case PN_TRANSPORT_ERROR: 
>  
> 1052 {
>  
> 1053 pn_transport_t *transport = pn_event_transport(e);   
>  
> 1054 pn_condition_t* condition = transport ? 
> pn_transport_condition(transport) : NULL; 
> 1055 if (ctx && ctx->connector) { /* Outgoing connection */   
>  
> 1056 qd_increment_conn_index(ctx);
>  
> 1057 const qd_server_config_t *config = 
> &ctx->connector->config;   
> 1058 ctx->connector->state = CXTR_STATE_FAILED;   
>  
> 1059 char conn_msg[300];  
>  
> 1060 if (condition  && pn_condition_is_set(condition)) {  
>  

[jira] [Commented] (DISPATCH-2046) Panic in handle due to deleted connector.

2021-04-27 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/DISPATCH-2046?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17333406#comment-17333406
 ] 

ASF GitHub Bot commented on DISPATCH-2046:
--

ganeshmurthy commented on a change in pull request #1142:
URL: https://github.com/apache/qpid-dispatch/pull/1142#discussion_r621440564



##
File path: src/server.c
##
@@ -1372,6 +1391,7 @@ void qd_server_free(qd_server_t *qd_server)
 if (!qd_server) return;
 
 qd_http_server_free(qd_server->http);
+qd_timer_visit(true);

Review comment:
   Take this scenario where a client closes its connection, the 
qd_connection_free() is called which schedules a timer here - 
https://github.com/apache/qpid-dispatch/blob/main/src/server.c#L917
   This newly scheduled timer is set to go off in ctx->connector->delay seconds 
(maybe 5 seconds or so). This timer gets added to the scheduled_timers in 
timer.c. The router shuts down before the timer can fire. We need to step thru 
the scheduled_timers and take care of all the timers which is what I am trying 
to do here. Once the timers are scheduled, how can the owner access the timer? 
I am sorry if that is something rudimentary I missed.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


> Panic in handle due to deleted connector.
> -
>
> Key: DISPATCH-2046
> URL: https://issues.apache.org/jira/browse/DISPATCH-2046
> Project: Qpid Dispatch
>  Issue Type: Bug
>  Components: Routing Engine
>Affects Versions: 1.15.0
>Reporter: Alex Ward
>Priority: Major
> Fix For: 1.17.0
>
>
> I am seeing a segv at line 1063 due to ctx->connector being 0 in 
> server.c:handle(). 
> {code:java}
> (ugdb-x86_64-7.11-261) p ctx
> $6 = (qd_connection_t *) 0x802e55890
> (ugdb-x86_64-7.11-261) p *ctx
> $7 = {prev = 0x8023df490, next = 0x802e7a890, name = 0x0, server = 
> 0x80220d0c0, opened = false, closed = false, closed_locally = false, enqueued 
> = 0, timer = 0x0, pn_conn = 0x8023ec050, pn_sessions = {0x0  times>}, ssl = 0x0, listener = 0x0, connector = 0x0, connector_lock = 
> 0x8023e6600, context = 0x0, user_context = 0x0, link_context = 0x0, 
> connection_id = 12, user_id = 0x0, free_user_id = false, policy_settings = 
> 0x0, n_sessions = 0, n_senders = 0, n_receivers = 0, open_container = 0x0, 
> deferred_calls = {head = 0x0, tail = 0x0, scratch = 0x0, size = 0}, 
> deferred_call_lock = 0x8023e65c0, policy_counted = false, role = 0x801a17660 
> "route-container", free_link_session_list = {head = 0x0, tail = 0x0, scratch 
> = 0x0, size = 0}, strip_annotations_in = false, strip_annotations_out = 
> false, wake = 0x8005d6e20 , rhost = '\000'  times>, rhost_port = '\000' }
> {code}
> As the comment at the start of handle() states, there is only one event being 
> handled at a time, but it doesn’t look like there is anything to stop someone 
> clearing ctx->connector between lines 1055 and 1063. The python code seems to 
> be coming in and deleting the connector while handle() is using it.
> [https://github.com/apache/qpid-dispatch/blob/main/src/server.c]
> {code:java}
>  989 /* Events involving a connection or listener are serialized by the 
> proactor so
>  990  * only one event per connection / listener will be processed at a time. 
>  
>  991  */  
>  992 static bool handle(qd_server_t *qd_server, pn_event_t *e, 
> pn_connection_t *pn_conn, qd_connection_t *ctx) 
>  993 {
>  
>  994 if (pn_conn && qdr_is_authentication_service_connection(pn_conn)) {  
>  
>  995 qdr_handle_authentication_service_connection_event(e);   
>  
>  996 return true; 
>  
>  997 }
>  
>  998  
>   
>  999 switch (pn_event_type(e)) { 
>  …
> 1051 case PN_TRANSPORT_ERROR: 
>  
> 1052 {
>  
> 1053 pn_transport_t *transport = pn_event_transport(e);   
>  
> 1054 pn_condition_t* condit

[jira] [Commented] (DISPATCH-2046) Panic in handle due to deleted connector.

2021-04-27 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/DISPATCH-2046?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=1784#comment-1784
 ] 

ASF GitHub Bot commented on DISPATCH-2046:
--

kgiusti commented on a change in pull request #1142:
URL: https://github.com/apache/qpid-dispatch/pull/1142#discussion_r621254289



##
File path: src/server.c
##
@@ -1372,6 +1391,7 @@ void qd_server_free(qd_server_t *qd_server)
 if (!qd_server) return;
 
 qd_http_server_free(qd_server->http);
+qd_timer_visit(true);

Review comment:
   I don't think this is the correct approach because this will mask 
leaking timers.
   
   If there are any timers still around when the router is being shutdown that 
means the owner of those timers never got around to properly cleaning them up.  
 That's a bug which will be hidden with the above patch.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


> Panic in handle due to deleted connector.
> -
>
> Key: DISPATCH-2046
> URL: https://issues.apache.org/jira/browse/DISPATCH-2046
> Project: Qpid Dispatch
>  Issue Type: Bug
>  Components: Routing Engine
>Affects Versions: 1.15.0
>Reporter: Alex Ward
>Priority: Major
> Fix For: 1.17.0
>
>
> I am seeing a segv at line 1063 due to ctx->connector being 0 in 
> server.c:handle(). 
> {code:java}
> (ugdb-x86_64-7.11-261) p ctx
> $6 = (qd_connection_t *) 0x802e55890
> (ugdb-x86_64-7.11-261) p *ctx
> $7 = {prev = 0x8023df490, next = 0x802e7a890, name = 0x0, server = 
> 0x80220d0c0, opened = false, closed = false, closed_locally = false, enqueued 
> = 0, timer = 0x0, pn_conn = 0x8023ec050, pn_sessions = {0x0  times>}, ssl = 0x0, listener = 0x0, connector = 0x0, connector_lock = 
> 0x8023e6600, context = 0x0, user_context = 0x0, link_context = 0x0, 
> connection_id = 12, user_id = 0x0, free_user_id = false, policy_settings = 
> 0x0, n_sessions = 0, n_senders = 0, n_receivers = 0, open_container = 0x0, 
> deferred_calls = {head = 0x0, tail = 0x0, scratch = 0x0, size = 0}, 
> deferred_call_lock = 0x8023e65c0, policy_counted = false, role = 0x801a17660 
> "route-container", free_link_session_list = {head = 0x0, tail = 0x0, scratch 
> = 0x0, size = 0}, strip_annotations_in = false, strip_annotations_out = 
> false, wake = 0x8005d6e20 , rhost = '\000'  times>, rhost_port = '\000' }
> {code}
> As the comment at the start of handle() states, there is only one event being 
> handled at a time, but it doesn’t look like there is anything to stop someone 
> clearing ctx->connector between lines 1055 and 1063. The python code seems to 
> be coming in and deleting the connector while handle() is using it.
> [https://github.com/apache/qpid-dispatch/blob/main/src/server.c]
> {code:java}
>  989 /* Events involving a connection or listener are serialized by the 
> proactor so
>  990  * only one event per connection / listener will be processed at a time. 
>  
>  991  */  
>  992 static bool handle(qd_server_t *qd_server, pn_event_t *e, 
> pn_connection_t *pn_conn, qd_connection_t *ctx) 
>  993 {
>  
>  994 if (pn_conn && qdr_is_authentication_service_connection(pn_conn)) {  
>  
>  995 qdr_handle_authentication_service_connection_event(e);   
>  
>  996 return true; 
>  
>  997 }
>  
>  998  
>   
>  999 switch (pn_event_type(e)) { 
>  …
> 1051 case PN_TRANSPORT_ERROR: 
>  
> 1052 {
>  
> 1053 pn_transport_t *transport = pn_event_transport(e);   
>  
> 1054 pn_condition_t* condition = transport ? 
> pn_transport_condition(transport) : NULL; 
> 1055 if (ctx && ctx->connector) { /* Outgoing connection */   
>  
> 1056 qd_increment_conn_index(ctx);
>  
> 1057 const qd_s

[jira] [Commented] (DISPATCH-2046) Panic in handle due to deleted connector.

2021-04-22 Thread Jira


[ 
https://issues.apache.org/jira/browse/DISPATCH-2046?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17327247#comment-17327247
 ] 

Jiri Daněk commented on DISPATCH-2046:
--

[~gmurthy] I might've seen the issue this ticket is describing happen on macOS. 
https://travis-ci.com/github/apache/qpid-dispatch/jobs/500355035#L4493 It is 
not something that's deterministically reproducible for me, so I won't be 
trying your patch, but "good timing coming up with the fix", I guess.

{noformat}
45: ==6605==ERROR: AddressSanitizer: SEGV on unknown address 0x (pc 
0x00010c0d4dfc bp 0x7925fcf0 sp 0x7925fc80 T2)
45: ==6605==The signal is caused by a READ memory access.
45: ==6605==Hint: address points to the zero page.
45: #0 0x10c0d4dfb in qd_connection_invoke_deferred server.c:1506
45: #1 0x10be818f0 in qd_connection_manager_delete_connector 
connection_manager.c:1060
45: #2 0x10e44fef4 in ffi_call_unix64 (libffi.7.dylib:x86_64+0x4ef4)
45: #3 0x7925fe4f ()
{noformat}

> Panic in handle due to deleted connector.
> -
>
> Key: DISPATCH-2046
> URL: https://issues.apache.org/jira/browse/DISPATCH-2046
> Project: Qpid Dispatch
>  Issue Type: Bug
>  Components: Routing Engine
>Affects Versions: 1.15.0
>Reporter: Alex Ward
>Priority: Major
> Fix For: 1.16.0
>
>
> I am seeing a segv at line 1063 due to ctx->connector being 0 in 
> server.c:handle(). 
> {code:java}
> (ugdb-x86_64-7.11-261) p ctx
> $6 = (qd_connection_t *) 0x802e55890
> (ugdb-x86_64-7.11-261) p *ctx
> $7 = {prev = 0x8023df490, next = 0x802e7a890, name = 0x0, server = 
> 0x80220d0c0, opened = false, closed = false, closed_locally = false, enqueued 
> = 0, timer = 0x0, pn_conn = 0x8023ec050, pn_sessions = {0x0  times>}, ssl = 0x0, listener = 0x0, connector = 0x0, connector_lock = 
> 0x8023e6600, context = 0x0, user_context = 0x0, link_context = 0x0, 
> connection_id = 12, user_id = 0x0, free_user_id = false, policy_settings = 
> 0x0, n_sessions = 0, n_senders = 0, n_receivers = 0, open_container = 0x0, 
> deferred_calls = {head = 0x0, tail = 0x0, scratch = 0x0, size = 0}, 
> deferred_call_lock = 0x8023e65c0, policy_counted = false, role = 0x801a17660 
> "route-container", free_link_session_list = {head = 0x0, tail = 0x0, scratch 
> = 0x0, size = 0}, strip_annotations_in = false, strip_annotations_out = 
> false, wake = 0x8005d6e20 , rhost = '\000'  times>, rhost_port = '\000' }
> {code}
> As the comment at the start of handle() states, there is only one event being 
> handled at a time, but it doesn’t look like there is anything to stop someone 
> clearing ctx->connector between lines 1055 and 1063. The python code seems to 
> be coming in and deleting the connector while handle() is using it.
> [https://github.com/apache/qpid-dispatch/blob/main/src/server.c]
> {code:java}
>  989 /* Events involving a connection or listener are serialized by the 
> proactor so
>  990  * only one event per connection / listener will be processed at a time. 
>  
>  991  */  
>  992 static bool handle(qd_server_t *qd_server, pn_event_t *e, 
> pn_connection_t *pn_conn, qd_connection_t *ctx) 
>  993 {
>  
>  994 if (pn_conn && qdr_is_authentication_service_connection(pn_conn)) {  
>  
>  995 qdr_handle_authentication_service_connection_event(e);   
>  
>  996 return true; 
>  
>  997 }
>  
>  998  
>   
>  999 switch (pn_event_type(e)) { 
>  …
> 1051 case PN_TRANSPORT_ERROR: 
>  
> 1052 {
>  
> 1053 pn_transport_t *transport = pn_event_transport(e);   
>  
> 1054 pn_condition_t* condition = transport ? 
> pn_transport_condition(transport) : NULL; 
> 1055 if (ctx && ctx->connector) { /* Outgoing connection */   
>  
> 1056 qd_increment_conn_index(ctx);
>  
> 1057 const qd_server_config_t *config = 
> &ctx->connector->config;   
> 1058 ctx->connector->s

[jira] [Commented] (DISPATCH-2046) Panic in handle due to deleted connector.

2021-04-20 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/DISPATCH-2046?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17326148#comment-17326148
 ] 

ASF GitHub Bot commented on DISPATCH-2046:
--

codecov-commenter commented on pull request #1142:
URL: https://github.com/apache/qpid-dispatch/pull/1142#issuecomment-823658233


   # 
[Codecov](https://codecov.io/gh/apache/qpid-dispatch/pull/1142?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
 Report
   > Merging 
[#1142](https://codecov.io/gh/apache/qpid-dispatch/pull/1142?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
 (cc9f870) into 
[main](https://codecov.io/gh/apache/qpid-dispatch/commit/b919a638e75ce42b0561b70671127b4e51f95d28?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
 (b919a63) will **increase** coverage by `0.01%`.
   > The diff coverage is `100.00%`.
   
   [![Impacted file tree 
graph](https://codecov.io/gh/apache/qpid-dispatch/pull/1142/graphs/tree.svg?width=650&height=150&src=pr&token=rk2Cgd27pP&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/qpid-dispatch/pull/1142?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@Coverage Diff @@
   ## main#1142  +/-   ##
   ==
   + Coverage   84.22%   84.23%   +0.01% 
   ==
 Files 111  111  
 Lines   2756927576   +7 
   ==
   + Hits2322023230  +10 
   + Misses   4349 4346   -3 
   ```
   
   
   | [Impacted 
Files](https://codecov.io/gh/apache/qpid-dispatch/pull/1142?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
 | Coverage Δ | |
   |---|---|---|
   | 
[src/connection\_manager.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1142/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL2Nvbm5lY3Rpb25fbWFuYWdlci5j)
 | `90.13% <100.00%> (ø)` | |
   | 
[src/server.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1142/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL3NlcnZlci5j)
 | `86.86% <100.00%> (+0.07%)` | :arrow_up: |
   | 
[src/router\_core/delivery.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1142/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL3JvdXRlcl9jb3JlL2RlbGl2ZXJ5LmM=)
 | `92.93% <0.00%> (-0.20%)` | :arrow_down: |
   | 
[src/iterator.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1142/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL2l0ZXJhdG9yLmM=)
 | `89.29% <0.00%> (-0.19%)` | :arrow_down: |
   | 
[src/message.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1142/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL21lc3NhZ2UuYw==)
 | `86.67% <0.00%> (-0.15%)` | :arrow_down: |
   | 
[src/router\_core/connections.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1142/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL3JvdXRlcl9jb3JlL2Nvbm5lY3Rpb25zLmM=)
 | `89.61% <0.00%> (ø)` | |
   | 
[src/hash.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1142/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL2hhc2guYw==)
 | `79.53% <0.00%> (+0.19%)` | :arrow_up: |
   | 
[src/adaptors/tcp\_adaptor.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1142/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL2FkYXB0b3JzL3RjcF9hZGFwdG9yLmM=)
 | `69.39% <0.00%> (+0.50%)` | :arrow_up: |
   | 
[src/router\_core/transfer.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1142/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#

[jira] [Commented] (DISPATCH-2046) Panic in handle due to deleted connector.

2021-04-20 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/DISPATCH-2046?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17326131#comment-17326131
 ] 

ASF GitHub Bot commented on DISPATCH-2046:
--

ganeshmurthy opened a new pull request #1142:
URL: https://github.com/apache/qpid-dispatch/pull/1142


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


> Panic in handle due to deleted connector.
> -
>
> Key: DISPATCH-2046
> URL: https://issues.apache.org/jira/browse/DISPATCH-2046
> Project: Qpid Dispatch
>  Issue Type: Bug
>  Components: Routing Engine
>Affects Versions: 1.15.0
>Reporter: Alex Ward
>Priority: Major
> Fix For: 1.16.0
>
>
> I am seeing a segv at line 1063 due to ctx->connector being 0 in 
> server.c:handle(). 
> {code:java}
> (ugdb-x86_64-7.11-261) p ctx
> $6 = (qd_connection_t *) 0x802e55890
> (ugdb-x86_64-7.11-261) p *ctx
> $7 = {prev = 0x8023df490, next = 0x802e7a890, name = 0x0, server = 
> 0x80220d0c0, opened = false, closed = false, closed_locally = false, enqueued 
> = 0, timer = 0x0, pn_conn = 0x8023ec050, pn_sessions = {0x0  times>}, ssl = 0x0, listener = 0x0, connector = 0x0, connector_lock = 
> 0x8023e6600, context = 0x0, user_context = 0x0, link_context = 0x0, 
> connection_id = 12, user_id = 0x0, free_user_id = false, policy_settings = 
> 0x0, n_sessions = 0, n_senders = 0, n_receivers = 0, open_container = 0x0, 
> deferred_calls = {head = 0x0, tail = 0x0, scratch = 0x0, size = 0}, 
> deferred_call_lock = 0x8023e65c0, policy_counted = false, role = 0x801a17660 
> "route-container", free_link_session_list = {head = 0x0, tail = 0x0, scratch 
> = 0x0, size = 0}, strip_annotations_in = false, strip_annotations_out = 
> false, wake = 0x8005d6e20 , rhost = '\000'  times>, rhost_port = '\000' }
> {code}
> As the comment at the start of handle() states, there is only one event being 
> handled at a time, but it doesn’t look like there is anything to stop someone 
> clearing ctx->connector between lines 1055 and 1063. The python code seems to 
> be coming in and deleting the connector while handle() is using it.
> [https://github.com/apache/qpid-dispatch/blob/main/src/server.c]
> {code:java}
>  989 /* Events involving a connection or listener are serialized by the 
> proactor so
>  990  * only one event per connection / listener will be processed at a time. 
>  
>  991  */  
>  992 static bool handle(qd_server_t *qd_server, pn_event_t *e, 
> pn_connection_t *pn_conn, qd_connection_t *ctx) 
>  993 {
>  
>  994 if (pn_conn && qdr_is_authentication_service_connection(pn_conn)) {  
>  
>  995 qdr_handle_authentication_service_connection_event(e);   
>  
>  996 return true; 
>  
>  997 }
>  
>  998  
>   
>  999 switch (pn_event_type(e)) { 
>  …
> 1051 case PN_TRANSPORT_ERROR: 
>  
> 1052 {
>  
> 1053 pn_transport_t *transport = pn_event_transport(e);   
>  
> 1054 pn_condition_t* condition = transport ? 
> pn_transport_condition(transport) : NULL; 
> 1055 if (ctx && ctx->connector) { /* Outgoing connection */   
>  
> 1056 qd_increment_conn_index(ctx);
>  
> 1057 const qd_server_config_t *config = 
> &ctx->connector->config;   
> 1058 ctx->connector->state = CXTR_STATE_FAILED;   
>  
> 1059 char conn_msg[300];  
>  
> 1060 if (condition  && pn_condition_is_set(condition)) {  
>  
> 1061 qd_format_string(conn_msg, 300, "[C%"PRIu64"] 
> Connection to %s failed: %s %s", ctx->connection_id, config

[jira] [Commented] (DISPATCH-2046) Panic in handle due to deleted connector.

2021-04-20 Thread Alex Ward (Jira)


[ 
https://issues.apache.org/jira/browse/DISPATCH-2046?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17325848#comment-17325848
 ] 

Alex Ward commented on DISPATCH-2046:
-

Great.  My sanity check testing passed without issue.  I'll be doing some more 
automated tests today but the change looks good.

Thanks Ganesh.

> Panic in handle due to deleted connector.
> -
>
> Key: DISPATCH-2046
> URL: https://issues.apache.org/jira/browse/DISPATCH-2046
> Project: Qpid Dispatch
>  Issue Type: Bug
>  Components: Routing Engine
>Affects Versions: 1.15.0
>Reporter: Alex Ward
>Priority: Major
> Fix For: 1.16.0
>
>
> I am seeing a segv at line 1063 due to ctx->connector being 0 in 
> server.c:handle(). 
> {code:java}
> (ugdb-x86_64-7.11-261) p ctx
> $6 = (qd_connection_t *) 0x802e55890
> (ugdb-x86_64-7.11-261) p *ctx
> $7 = {prev = 0x8023df490, next = 0x802e7a890, name = 0x0, server = 
> 0x80220d0c0, opened = false, closed = false, closed_locally = false, enqueued 
> = 0, timer = 0x0, pn_conn = 0x8023ec050, pn_sessions = {0x0  times>}, ssl = 0x0, listener = 0x0, connector = 0x0, connector_lock = 
> 0x8023e6600, context = 0x0, user_context = 0x0, link_context = 0x0, 
> connection_id = 12, user_id = 0x0, free_user_id = false, policy_settings = 
> 0x0, n_sessions = 0, n_senders = 0, n_receivers = 0, open_container = 0x0, 
> deferred_calls = {head = 0x0, tail = 0x0, scratch = 0x0, size = 0}, 
> deferred_call_lock = 0x8023e65c0, policy_counted = false, role = 0x801a17660 
> "route-container", free_link_session_list = {head = 0x0, tail = 0x0, scratch 
> = 0x0, size = 0}, strip_annotations_in = false, strip_annotations_out = 
> false, wake = 0x8005d6e20 , rhost = '\000'  times>, rhost_port = '\000' }
> {code}
> As the comment at the start of handle() states, there is only one event being 
> handled at a time, but it doesn’t look like there is anything to stop someone 
> clearing ctx->connector between lines 1055 and 1063. The python code seems to 
> be coming in and deleting the connector while handle() is using it.
> [https://github.com/apache/qpid-dispatch/blob/main/src/server.c]
> {code:java}
>  989 /* Events involving a connection or listener are serialized by the 
> proactor so
>  990  * only one event per connection / listener will be processed at a time. 
>  
>  991  */  
>  992 static bool handle(qd_server_t *qd_server, pn_event_t *e, 
> pn_connection_t *pn_conn, qd_connection_t *ctx) 
>  993 {
>  
>  994 if (pn_conn && qdr_is_authentication_service_connection(pn_conn)) {  
>  
>  995 qdr_handle_authentication_service_connection_event(e);   
>  
>  996 return true; 
>  
>  997 }
>  
>  998  
>   
>  999 switch (pn_event_type(e)) { 
>  …
> 1051 case PN_TRANSPORT_ERROR: 
>  
> 1052 {
>  
> 1053 pn_transport_t *transport = pn_event_transport(e);   
>  
> 1054 pn_condition_t* condition = transport ? 
> pn_transport_condition(transport) : NULL; 
> 1055 if (ctx && ctx->connector) { /* Outgoing connection */   
>  
> 1056 qd_increment_conn_index(ctx);
>  
> 1057 const qd_server_config_t *config = 
> &ctx->connector->config;   
> 1058 ctx->connector->state = CXTR_STATE_FAILED;   
>  
> 1059 char conn_msg[300];  
>  
> 1060 if (condition  && pn_condition_is_set(condition)) {  
>  
> 1061 qd_format_string(conn_msg, 300, "[C%"PRIu64"] 
> Connection to %s failed: %s %s", ctx->connection_id, config->host_port,   
> 
> 1062 pn_condition_get_name(condition), 
> pn_condition_get_description(condition));   
> 1063 

[jira] [Commented] (DISPATCH-2046) Panic in handle due to deleted connector.

2021-04-19 Thread Ganesh Murthy (Jira)


[ 
https://issues.apache.org/jira/browse/DISPATCH-2046?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17325440#comment-17325440
 ] 

Ganesh Murthy commented on DISPATCH-2046:
-

Thanks for your comment. I included your change here - 
https://github.com/ganeshmurthy/qpid-dispatch/commit/67cc0a38d29f4161d5a3f9f18dfb0ce6e239

Please take a look, it is passing all tests. Thanks.

> Panic in handle due to deleted connector.
> -
>
> Key: DISPATCH-2046
> URL: https://issues.apache.org/jira/browse/DISPATCH-2046
> Project: Qpid Dispatch
>  Issue Type: Bug
>  Components: Routing Engine
>Affects Versions: 1.15.0
>Reporter: Alex Ward
>Priority: Major
> Fix For: 1.16.0
>
>
> I am seeing a segv at line 1063 due to ctx->connector being 0 in 
> server.c:handle(). 
> {code:java}
> (ugdb-x86_64-7.11-261) p ctx
> $6 = (qd_connection_t *) 0x802e55890
> (ugdb-x86_64-7.11-261) p *ctx
> $7 = {prev = 0x8023df490, next = 0x802e7a890, name = 0x0, server = 
> 0x80220d0c0, opened = false, closed = false, closed_locally = false, enqueued 
> = 0, timer = 0x0, pn_conn = 0x8023ec050, pn_sessions = {0x0  times>}, ssl = 0x0, listener = 0x0, connector = 0x0, connector_lock = 
> 0x8023e6600, context = 0x0, user_context = 0x0, link_context = 0x0, 
> connection_id = 12, user_id = 0x0, free_user_id = false, policy_settings = 
> 0x0, n_sessions = 0, n_senders = 0, n_receivers = 0, open_container = 0x0, 
> deferred_calls = {head = 0x0, tail = 0x0, scratch = 0x0, size = 0}, 
> deferred_call_lock = 0x8023e65c0, policy_counted = false, role = 0x801a17660 
> "route-container", free_link_session_list = {head = 0x0, tail = 0x0, scratch 
> = 0x0, size = 0}, strip_annotations_in = false, strip_annotations_out = 
> false, wake = 0x8005d6e20 , rhost = '\000'  times>, rhost_port = '\000' }
> {code}
> As the comment at the start of handle() states, there is only one event being 
> handled at a time, but it doesn’t look like there is anything to stop someone 
> clearing ctx->connector between lines 1055 and 1063. The python code seems to 
> be coming in and deleting the connector while handle() is using it.
> [https://github.com/apache/qpid-dispatch/blob/main/src/server.c]
> {code:java}
>  989 /* Events involving a connection or listener are serialized by the 
> proactor so
>  990  * only one event per connection / listener will be processed at a time. 
>  
>  991  */  
>  992 static bool handle(qd_server_t *qd_server, pn_event_t *e, 
> pn_connection_t *pn_conn, qd_connection_t *ctx) 
>  993 {
>  
>  994 if (pn_conn && qdr_is_authentication_service_connection(pn_conn)) {  
>  
>  995 qdr_handle_authentication_service_connection_event(e);   
>  
>  996 return true; 
>  
>  997 }
>  
>  998  
>   
>  999 switch (pn_event_type(e)) { 
>  …
> 1051 case PN_TRANSPORT_ERROR: 
>  
> 1052 {
>  
> 1053 pn_transport_t *transport = pn_event_transport(e);   
>  
> 1054 pn_condition_t* condition = transport ? 
> pn_transport_condition(transport) : NULL; 
> 1055 if (ctx && ctx->connector) { /* Outgoing connection */   
>  
> 1056 qd_increment_conn_index(ctx);
>  
> 1057 const qd_server_config_t *config = 
> &ctx->connector->config;   
> 1058 ctx->connector->state = CXTR_STATE_FAILED;   
>  
> 1059 char conn_msg[300];  
>  
> 1060 if (condition  && pn_condition_is_set(condition)) {  
>  
> 1061 qd_format_string(conn_msg, 300, "[C%"PRIu64"] 
> Connection to %s failed: %s %s", ctx->connection_id, config->host_port,   
> 
> 1062 pn_condition_get_name(condition), 
> p

[jira] [Commented] (DISPATCH-2046) Panic in handle due to deleted connector.

2021-04-19 Thread Alex Ward (Jira)


[ 
https://issues.apache.org/jira/browse/DISPATCH-2046?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17325251#comment-17325251
 ] 

Alex Ward commented on DISPATCH-2046:
-

I did have one comment on your change, I added it to the github.  I'm merging 
your change with our code and should be able to do some testing tomorrow,  

> Panic in handle due to deleted connector.
> -
>
> Key: DISPATCH-2046
> URL: https://issues.apache.org/jira/browse/DISPATCH-2046
> Project: Qpid Dispatch
>  Issue Type: Bug
>  Components: Routing Engine
>Affects Versions: 1.15.0
>Reporter: Alex Ward
>Priority: Major
> Fix For: 1.16.0
>
>
> I am seeing a segv at line 1063 due to ctx->connector being 0 in 
> server.c:handle(). 
> {code:java}
> (ugdb-x86_64-7.11-261) p ctx
> $6 = (qd_connection_t *) 0x802e55890
> (ugdb-x86_64-7.11-261) p *ctx
> $7 = {prev = 0x8023df490, next = 0x802e7a890, name = 0x0, server = 
> 0x80220d0c0, opened = false, closed = false, closed_locally = false, enqueued 
> = 0, timer = 0x0, pn_conn = 0x8023ec050, pn_sessions = {0x0  times>}, ssl = 0x0, listener = 0x0, connector = 0x0, connector_lock = 
> 0x8023e6600, context = 0x0, user_context = 0x0, link_context = 0x0, 
> connection_id = 12, user_id = 0x0, free_user_id = false, policy_settings = 
> 0x0, n_sessions = 0, n_senders = 0, n_receivers = 0, open_container = 0x0, 
> deferred_calls = {head = 0x0, tail = 0x0, scratch = 0x0, size = 0}, 
> deferred_call_lock = 0x8023e65c0, policy_counted = false, role = 0x801a17660 
> "route-container", free_link_session_list = {head = 0x0, tail = 0x0, scratch 
> = 0x0, size = 0}, strip_annotations_in = false, strip_annotations_out = 
> false, wake = 0x8005d6e20 , rhost = '\000'  times>, rhost_port = '\000' }
> {code}
> As the comment at the start of handle() states, there is only one event being 
> handled at a time, but it doesn’t look like there is anything to stop someone 
> clearing ctx->connector between lines 1055 and 1063. The python code seems to 
> be coming in and deleting the connector while handle() is using it.
> [https://github.com/apache/qpid-dispatch/blob/main/src/server.c]
> {code:java}
>  989 /* Events involving a connection or listener are serialized by the 
> proactor so
>  990  * only one event per connection / listener will be processed at a time. 
>  
>  991  */  
>  992 static bool handle(qd_server_t *qd_server, pn_event_t *e, 
> pn_connection_t *pn_conn, qd_connection_t *ctx) 
>  993 {
>  
>  994 if (pn_conn && qdr_is_authentication_service_connection(pn_conn)) {  
>  
>  995 qdr_handle_authentication_service_connection_event(e);   
>  
>  996 return true; 
>  
>  997 }
>  
>  998  
>   
>  999 switch (pn_event_type(e)) { 
>  …
> 1051 case PN_TRANSPORT_ERROR: 
>  
> 1052 {
>  
> 1053 pn_transport_t *transport = pn_event_transport(e);   
>  
> 1054 pn_condition_t* condition = transport ? 
> pn_transport_condition(transport) : NULL; 
> 1055 if (ctx && ctx->connector) { /* Outgoing connection */   
>  
> 1056 qd_increment_conn_index(ctx);
>  
> 1057 const qd_server_config_t *config = 
> &ctx->connector->config;   
> 1058 ctx->connector->state = CXTR_STATE_FAILED;   
>  
> 1059 char conn_msg[300];  
>  
> 1060 if (condition  && pn_condition_is_set(condition)) {  
>  
> 1061 qd_format_string(conn_msg, 300, "[C%"PRIu64"] 
> Connection to %s failed: %s %s", ctx->connection_id, config->host_port,   
> 
> 1062 pn_condition_get_name(condition), 
> pn_condition_get_description(condition));   
> 1063   

[jira] [Commented] (DISPATCH-2046) Panic in handle due to deleted connector.

2021-04-16 Thread Alex Ward (Jira)


[ 
https://issues.apache.org/jira/browse/DISPATCH-2046?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17323915#comment-17323915
 ] 

Alex Ward commented on DISPATCH-2046:
-

Thanks Ganesh, I'll try that on our code too, let me know how your testing goes.

> Panic in handle due to deleted connector.
> -
>
> Key: DISPATCH-2046
> URL: https://issues.apache.org/jira/browse/DISPATCH-2046
> Project: Qpid Dispatch
>  Issue Type: Bug
>  Components: Routing Engine
>Affects Versions: 1.15.0
>Reporter: Alex Ward
>Priority: Major
>
> I am seeing a segv at line 1063 due to ctx->connector being 0 in 
> server.c:handle(). 
> {code:java}
> (ugdb-x86_64-7.11-261) p ctx
> $6 = (qd_connection_t *) 0x802e55890
> (ugdb-x86_64-7.11-261) p *ctx
> $7 = {prev = 0x8023df490, next = 0x802e7a890, name = 0x0, server = 
> 0x80220d0c0, opened = false, closed = false, closed_locally = false, enqueued 
> = 0, timer = 0x0, pn_conn = 0x8023ec050, pn_sessions = {0x0  times>}, ssl = 0x0, listener = 0x0, connector = 0x0, connector_lock = 
> 0x8023e6600, context = 0x0, user_context = 0x0, link_context = 0x0, 
> connection_id = 12, user_id = 0x0, free_user_id = false, policy_settings = 
> 0x0, n_sessions = 0, n_senders = 0, n_receivers = 0, open_container = 0x0, 
> deferred_calls = {head = 0x0, tail = 0x0, scratch = 0x0, size = 0}, 
> deferred_call_lock = 0x8023e65c0, policy_counted = false, role = 0x801a17660 
> "route-container", free_link_session_list = {head = 0x0, tail = 0x0, scratch 
> = 0x0, size = 0}, strip_annotations_in = false, strip_annotations_out = 
> false, wake = 0x8005d6e20 , rhost = '\000'  times>, rhost_port = '\000' }
> {code}
> As the comment at the start of handle() states, there is only one event being 
> handled at a time, but it doesn’t look like there is anything to stop someone 
> clearing ctx->connector between lines 1055 and 1063. The python code seems to 
> be coming in and deleting the connector while handle() is using it.
> [https://github.com/apache/qpid-dispatch/blob/main/src/server.c]
> {code:java}
>  989 /* Events involving a connection or listener are serialized by the 
> proactor so
>  990  * only one event per connection / listener will be processed at a time. 
>  
>  991  */  
>  992 static bool handle(qd_server_t *qd_server, pn_event_t *e, 
> pn_connection_t *pn_conn, qd_connection_t *ctx) 
>  993 {
>  
>  994 if (pn_conn && qdr_is_authentication_service_connection(pn_conn)) {  
>  
>  995 qdr_handle_authentication_service_connection_event(e);   
>  
>  996 return true; 
>  
>  997 }
>  
>  998  
>   
>  999 switch (pn_event_type(e)) { 
>  …
> 1051 case PN_TRANSPORT_ERROR: 
>  
> 1052 {
>  
> 1053 pn_transport_t *transport = pn_event_transport(e);   
>  
> 1054 pn_condition_t* condition = transport ? 
> pn_transport_condition(transport) : NULL; 
> 1055 if (ctx && ctx->connector) { /* Outgoing connection */   
>  
> 1056 qd_increment_conn_index(ctx);
>  
> 1057 const qd_server_config_t *config = 
> &ctx->connector->config;   
> 1058 ctx->connector->state = CXTR_STATE_FAILED;   
>  
> 1059 char conn_msg[300];  
>  
> 1060 if (condition  && pn_condition_is_set(condition)) {  
>  
> 1061 qd_format_string(conn_msg, 300, "[C%"PRIu64"] 
> Connection to %s failed: %s %s", ctx->connection_id, config->host_port,   
> 
> 1062 pn_condition_get_name(condition), 
> pn_condition_get_description(condition));   
> 1063 strcpy(ctx->connector->conn_msg, conn_msg); 
> {code}
> Another thread is at line 1063 i

[jira] [Commented] (DISPATCH-2046) Panic in handle due to deleted connector.

2021-04-15 Thread Ganesh Murthy (Jira)


[ 
https://issues.apache.org/jira/browse/DISPATCH-2046?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17322543#comment-17322543
 ] 

Ganesh Murthy commented on DISPATCH-2046:
-

I still have more testing to do but please take a look - 
[https://github.com/ganeshmurthy/qpid-dispatch/commit/2e663dc828f52ece9e5bac3b5d9342d74c43070c]

I completely removed the ct->lock and am using the already available 
server->lock.

I am still investigating to see if my patch introduces any new issues

> Panic in handle due to deleted connector.
> -
>
> Key: DISPATCH-2046
> URL: https://issues.apache.org/jira/browse/DISPATCH-2046
> Project: Qpid Dispatch
>  Issue Type: Bug
>  Components: Routing Engine
>Affects Versions: 1.15.0
>Reporter: Alex Ward
>Priority: Major
>
> I am seeing a segv at line 1063 due to ctx->connector being 0 in 
> server.c:handle(). 
> {code:java}
> (ugdb-x86_64-7.11-261) p ctx
> $6 = (qd_connection_t *) 0x802e55890
> (ugdb-x86_64-7.11-261) p *ctx
> $7 = {prev = 0x8023df490, next = 0x802e7a890, name = 0x0, server = 
> 0x80220d0c0, opened = false, closed = false, closed_locally = false, enqueued 
> = 0, timer = 0x0, pn_conn = 0x8023ec050, pn_sessions = {0x0  times>}, ssl = 0x0, listener = 0x0, connector = 0x0, connector_lock = 
> 0x8023e6600, context = 0x0, user_context = 0x0, link_context = 0x0, 
> connection_id = 12, user_id = 0x0, free_user_id = false, policy_settings = 
> 0x0, n_sessions = 0, n_senders = 0, n_receivers = 0, open_container = 0x0, 
> deferred_calls = {head = 0x0, tail = 0x0, scratch = 0x0, size = 0}, 
> deferred_call_lock = 0x8023e65c0, policy_counted = false, role = 0x801a17660 
> "route-container", free_link_session_list = {head = 0x0, tail = 0x0, scratch 
> = 0x0, size = 0}, strip_annotations_in = false, strip_annotations_out = 
> false, wake = 0x8005d6e20 , rhost = '\000'  times>, rhost_port = '\000' }
> {code}
> As the comment at the start of handle() states, there is only one event being 
> handled at a time, but it doesn’t look like there is anything to stop someone 
> clearing ctx->connector between lines 1055 and 1063. The python code seems to 
> be coming in and deleting the connector while handle() is using it.
> [https://github.com/apache/qpid-dispatch/blob/main/src/server.c]
> {code:java}
>  989 /* Events involving a connection or listener are serialized by the 
> proactor so
>  990  * only one event per connection / listener will be processed at a time. 
>  
>  991  */  
>  992 static bool handle(qd_server_t *qd_server, pn_event_t *e, 
> pn_connection_t *pn_conn, qd_connection_t *ctx) 
>  993 {
>  
>  994 if (pn_conn && qdr_is_authentication_service_connection(pn_conn)) {  
>  
>  995 qdr_handle_authentication_service_connection_event(e);   
>  
>  996 return true; 
>  
>  997 }
>  
>  998  
>   
>  999 switch (pn_event_type(e)) { 
>  …
> 1051 case PN_TRANSPORT_ERROR: 
>  
> 1052 {
>  
> 1053 pn_transport_t *transport = pn_event_transport(e);   
>  
> 1054 pn_condition_t* condition = transport ? 
> pn_transport_condition(transport) : NULL; 
> 1055 if (ctx && ctx->connector) { /* Outgoing connection */   
>  
> 1056 qd_increment_conn_index(ctx);
>  
> 1057 const qd_server_config_t *config = 
> &ctx->connector->config;   
> 1058 ctx->connector->state = CXTR_STATE_FAILED;   
>  
> 1059 char conn_msg[300];  
>  
> 1060 if (condition  && pn_condition_is_set(condition)) {  
>  
> 1061 qd_format_string(conn_msg, 300, "[C%"PRIu64"] 
> Connection to %s failed: %s %s", ctx->connection_id, config->host_port,   
> 

[jira] [Commented] (DISPATCH-2046) Panic in handle due to deleted connector.

2021-04-15 Thread Alex Ward (Jira)


[ 
https://issues.apache.org/jira/browse/DISPATCH-2046?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17322095#comment-17322095
 ] 

Alex Ward commented on DISPATCH-2046:
-

Any suggestions on resolving this race condition? Is there anything 
architectuarally that should be preventing the ctx->connection being modified 
by the python code?

There is a lock in qd_connector_t that seems to be to prevent this sort of thing

{{144 /* Connector state and ctx can be modified in proactor or management 
threads. */}}
{{145 sys_mutex_t *lock;}}

But that is not being held in this case.  Should it be somehow?  Should it be 
held going into handle()?

The connector lock is taken in the management code on line 1059 but nothing is 
protecting the connection.  Does the connection need it's own lock so the code 
would look like:

{{1055 void qd_connection_manager_delete_connector(qd_dispatch_t *qd, void 
*impl)}}
{{ 1056 { }}
{{ 1057     qd_connector_t ct = (qd_connector_t) impl; }}
{{ 1058     if (ct) { }}
{{ 1059         sys_mutex_lock(ct->lock); }}
{{ 1060         if (ct->ctx) {}}
{color:#FF}{{new              sys_mutex_lock(ct->ctx->lock); }}{color}
{{ 1061             ct->ctx->connector = 0; }}

 

 

> Panic in handle due to deleted connector.
> -
>
> Key: DISPATCH-2046
> URL: https://issues.apache.org/jira/browse/DISPATCH-2046
> Project: Qpid Dispatch
>  Issue Type: Bug
>  Components: Routing Engine
>Affects Versions: 1.15.0
>Reporter: Alex Ward
>Priority: Major
>
> I am seeing a segv at line 1063 due to ctx->connector being 0 in 
> server.c:handle().
> ugdb-x86_64-7.11-261) p *ctx
> $7 = {prev = 0x8023df490, next = 0x802e7a890, name = 0x0, server = 
> 0x80220d0c0, opened = false, closed = false, closed_locally = false, enqueued 
> = 0, timer = 0x0, pn_conn = 0x8023ec050, pn_sessions = {0x0  times>}, ssl = 0x0, listener = 0x0, connector = 0x0, connector_lock = 
> 0x8023e6600, context = 0x0, user_context = 0x0, link_context = 0x0, 
> connection_id = 12, user_id = 0x0, free_user_id = false, policy_settings = 
> 0x0, n_sessions = 0, n_senders = 0, n_receivers = 0, open_container = 0x0, 
> deferred_calls = {head = 0x0, tail = 0x0, scratch = 0x0, size = 0}, 
> deferred_call_lock = 0x8023e65c0, policy_counted = false, role = 0x801a17660 
> "route-container", free_link_session_list = {head = 0x0, tail = 0x0, scratch 
> = 0x0, size = 0}, strip_annotations_in = false, strip_annotations_out = 
> false, wake = 0x8005d6e20 , rhost = '\000'  times>, rhost_port = '\000' }
> As the comment at the start of handle() states, there is only one event being 
> handled at a time, but it doesn’t look like there is anything to stop someone 
> clearing ctx->connector between lines 1055 and 1063. The python code seems to 
> be comin in and deleting the connector while handle is using it
> https://github.com/apache/qpid-dispatch/blob/main/src/server.c
>   
>
>  989 /* Events involving a connection or listener are serialized by the 
> proactor so
>  990  * only one event per connection / listener will be processed at a time. 
>  
>  991  */  
>  992 static bool handle(qd_server_t *qd_server, pn_event_t *e, 
> pn_connection_t *pn_conn, qd_connection_t *ctx) 
>  993 {
>  
>  994 if (pn_conn && qdr_is_authentication_service_connection(pn_conn)) {  
>  
>  995 qdr_handle_authentication_service_connection_event(e);   
>  
>  996 return true; 
>  
>  997 }
>  
>  998  
>   
>  999 switch (pn_event_type(e)) { 
>  …
> 1051 case PN_TRANSPORT_ERROR: 
>  
> 1052 {
>  
> 1053 pn_transport_t *transport = pn_event_transport(e);   
>  
> 1054 pn_condition_t* condition = transport ? 
> pn_transport_condition(transport) : NULL; 
> 1055 if (ctx && ctx->connector) { /* Outgoing connection */   
>  
> 1056 qd_increment_conn_index(ctx);
>