You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@qpid.apache.org by GitBox <gi...@apache.org> on 2021/04/20 22:36:41 UTC
[GitHub] [qpid-dispatch] ganeshmurthy opened a new pull request #1142: DISPATCH-2046: Removed the connector lock and used the server lock
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-dispatch] kgiusti commented on a change in pull request #1142: DISPATCH-2046: Removed the connector lock and used the server lock
Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-dispatch] ganeshmurthy commented on a change in pull request #1142: DISPATCH-2046: Removed the connector lock and used the server lock
Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-dispatch] codecov-commenter commented on pull request #1142: DISPATCH-2046: Removed the connector lock and used the server lock
Posted by GitBox <gi...@apache.org>.
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%`.
[](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 27569 27576 +7
==========================================
+ Hits 23220 23230 +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#diff-c3JjL3JvdXRlcl9jb3JlL3RyYW5zZmVyLmM=) | `94.18% <0.00%> (+0.64%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/qpid-dispatch/pull/1142?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/qpid-dispatch/pull/1142?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [b919a63...cc9f870](https://codecov.io/gh/apache/qpid-dispatch/pull/1142?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-dispatch] ganeshmurthy closed pull request #1142: DISPATCH-2046: Removed the connector lock and used the server lock
Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org