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/09/09 15:00:10 UTC

[GitHub] [qpid-dispatch] mgoulish opened a new pull request #1366: Rewrite of log source locking.

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


   * Individual locks for each log source -- no more global lock for log sources.
   * New global lock for recent log entries list.
   * A big comment explaining the new overall locking approach.


-- 
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.

To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org

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 #1366: Rewrite of log source locking.

Posted by GitBox <gi...@apache.org>.
kgiusti commented on a change in pull request #1366:
URL: https://github.com/apache/qpid-dispatch/pull/1366#discussion_r707362878



##########
File path: src/log.c
##########
@@ -59,12 +88,11 @@ struct qd_log_entry_t {
     struct timeval  time;
     char            text[TEXT_MAX];
 };
-
 ALLOC_DECLARE(qd_log_entry_t);
 ALLOC_DEFINE(qd_log_entry_t);
-
 DEQ_DECLARE(qd_log_entry_t, qd_log_list_t);
 static qd_log_list_t         entries = {0};
+sys_mutex_t * entries_lock = 0;

Review comment:
       code convention: remove space between * and entries_lock please




-- 
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.

To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org

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 edited a comment on pull request #1366: Rewrite of log source locking.

Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #1366:
URL: https://github.com/apache/qpid-dispatch/pull/1366#issuecomment-916326174


   # [Codecov](https://codecov.io/gh/apache/qpid-dispatch/pull/1366?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 [#1366](https://codecov.io/gh/apache/qpid-dispatch/pull/1366?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (9534ac4) into [main](https://codecov.io/gh/apache/qpid-dispatch/commit/db1170421c051e48defd2eee62be21da583021a1?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (db11704) will **increase** coverage by `0.08%`.
   > The diff coverage is `92.30%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/qpid-dispatch/pull/1366/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/1366?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    #1366      +/-   ##
   ==========================================
   + Coverage   84.62%   84.71%   +0.08%     
   ==========================================
     Files         114      114              
     Lines       28405    28414       +9     
   ==========================================
   + Hits        24038    24070      +32     
   + Misses       4367     4344      -23     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/qpid-dispatch/pull/1366?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/log.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1366/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-c3JjL2xvZy5j) | `91.16% <92.30%> (-0.04%)` | :arrow_down: |
   | [src/router\_core/connections.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1366/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.38% <0.00%> (-0.10%)` | :arrow_down: |
   | [src/router\_core/forwarder.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1366/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-c3JjL3JvdXRlcl9jb3JlL2ZvcndhcmRlci5j) | `93.05% <0.00%> (-0.02%)` | :arrow_down: |
   | [src/router\_core/transfer.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1366/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.60% <0.00%> (+0.85%)` | :arrow_up: |
   | [src/iterator.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1366/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=) | `93.35% <0.00%> (+3.89%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/qpid-dispatch/pull/1366?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/1366?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 [db11704...9534ac4](https://codecov.io/gh/apache/qpid-dispatch/pull/1366?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.

To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org

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] asfgit closed pull request #1366: Rewrite of log source locking.

Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #1366:
URL: https://github.com/apache/qpid-dispatch/pull/1366


   


-- 
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.

To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org

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 #1366: Rewrite of log source locking.

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #1366:
URL: https://github.com/apache/qpid-dispatch/pull/1366#issuecomment-916326174


   # [Codecov](https://codecov.io/gh/apache/qpid-dispatch/pull/1366?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 [#1366](https://codecov.io/gh/apache/qpid-dispatch/pull/1366?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (5e2278d) into [main](https://codecov.io/gh/apache/qpid-dispatch/commit/db1170421c051e48defd2eee62be21da583021a1?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (db11704) will **increase** coverage by `0.09%`.
   > The diff coverage is `93.87%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/qpid-dispatch/pull/1366/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/1366?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    #1366      +/-   ##
   ==========================================
   + Coverage   84.62%   84.71%   +0.09%     
   ==========================================
     Files         114      114              
     Lines       28405    28405              
   ==========================================
   + Hits        24038    24064      +26     
   + Misses       4367     4341      -26     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/qpid-dispatch/pull/1366?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/log.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1366/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-c3JjL2xvZy5j) | `91.81% <93.87%> (+0.61%)` | :arrow_up: |
   | [...router\_core/modules/edge\_router/link\_route\_proxy.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1366/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/router\_core/modules/edge\_router/edge\_mgmt.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1366/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/iterator.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1366/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.17%)` | :arrow_down: |
   | [src/router\_core/forwarder.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1366/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-c3JjL3JvdXRlcl9jb3JlL2ZvcndhcmRlci5j) | `93.05% <0.00%> (-0.02%)` | :arrow_down: |
   | [src/router\_node.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1366/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=) | `93.55% <0.00%> (+0.09%)` | :arrow_up: |
   | [src/adaptors/http1/http1\_server.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1366/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-c3JjL2FkYXB0b3JzL2h0dHAxL2h0dHAxX3NlcnZlci5j) | `85.71% <0.00%> (+0.28%)` | :arrow_up: |
   | [src/router\_core/transfer.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1366/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.16% <0.00%> (+0.41%)` | :arrow_up: |
   | [src/router\_core/delivery.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1366/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.90% <0.00%> (+0.55%)` | :arrow_up: |
   | [src/router\_core/connections.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1366/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=) | `90.26% <0.00%> (+0.78%)` | :arrow_up: |
   | ... and [1 more](https://codecov.io/gh/apache/qpid-dispatch/pull/1366/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/qpid-dispatch/pull/1366?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/1366?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 [db11704...5e2278d](https://codecov.io/gh/apache/qpid-dispatch/pull/1366?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.

To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org

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 edited a comment on pull request #1366: Rewrite of log source locking.

Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #1366:
URL: https://github.com/apache/qpid-dispatch/pull/1366#issuecomment-916326174


   # [Codecov](https://codecov.io/gh/apache/qpid-dispatch/pull/1366?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 [#1366](https://codecov.io/gh/apache/qpid-dispatch/pull/1366?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c8bcaf0) into [main](https://codecov.io/gh/apache/qpid-dispatch/commit/db1170421c051e48defd2eee62be21da583021a1?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (db11704) will **decrease** coverage by `0.01%`.
   > The diff coverage is `93.05%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/qpid-dispatch/pull/1366/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/1366?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    #1366      +/-   ##
   ==========================================
   - Coverage   84.62%   84.61%   -0.02%     
   ==========================================
     Files         114      114              
     Lines       28405    28409       +4     
   ==========================================
   - Hits        24038    24037       -1     
   - Misses       4367     4372       +5     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/qpid-dispatch/pull/1366?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/log.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1366/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-c3JjL2xvZy5j) | `91.32% <93.05%> (+0.12%)` | :arrow_up: |
   | [src/router\_core/router\_core.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1366/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.05% <0.00%> (-0.96%)` | :arrow_down: |
   | [src/router\_core/connections.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1366/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.18% <0.00%> (-0.30%)` | :arrow_down: |
   | [src/iterator.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1366/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.17%)` | :arrow_down: |
   | [src/message.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1366/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==) | `87.55% <0.00%> (+0.06%)` | :arrow_up: |
   | [src/router\_core/forwarder.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1366/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-c3JjL3JvdXRlcl9jb3JlL2ZvcndhcmRlci5j) | `93.45% <0.00%> (+0.38%)` | :arrow_up: |
   | [src/router\_core/transfer.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1366/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.16% <0.00%> (+0.41%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/qpid-dispatch/pull/1366?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/1366?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 [db11704...c8bcaf0](https://codecov.io/gh/apache/qpid-dispatch/pull/1366?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.

To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org

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 #1366: Rewrite of log source locking.

Posted by GitBox <gi...@apache.org>.
kgiusti commented on a change in pull request #1366:
URL: https://github.com/apache/qpid-dispatch/pull/1366#discussion_r707394576



##########
File path: src/log.c
##########
@@ -365,41 +390,21 @@ static void qd_log_source_defaults(qd_log_source_t *log_source) {
     memset ( log_source->severity_histogram, 0, sizeof(uint64_t) * (N_LEVEL_INDICES) );
 }
 
-/// Caller must hold the log_source_lock
-static qd_log_source_t *qd_log_source_lh(const char *module)
-{
-    qd_log_source_t *log_source = lookup_log_source_lh(module);
-    if (!log_source)
-    {
-        log_source = NEW(qd_log_source_t);
-        ZERO(log_source);
-        log_source->module = (char*) malloc(strlen(module) + 1);
-        strcpy(log_source->module, module);
-        qd_log_source_defaults(log_source);
-        DEQ_INSERT_TAIL(source_list, log_source);
-        qd_entity_cache_add(QD_LOG_STATS_TYPE, log_source);
-    }
-    return log_source;
-}
-
 qd_log_source_t *qd_log_source(const char *module)
 {
-    sys_mutex_lock(log_source_lock);
-    qd_log_source_t* src = qd_log_source_lh(module);
-    sys_mutex_unlock(log_source_lock);
+    qd_log_source_t* src = lookup_log_source(module);
     return src;
 }
 
 qd_log_source_t *qd_log_source_reset(const char *module)

Review comment:
       Issue: qd_log_source_defaults changes the log_source's data without holding the source's lock.   qd_log_source_reset() is called by the mgmt thread so it is modifying the log source without holding the lock.

##########
File path: src/log.c
##########
@@ -353,7 +378,7 @@ static void write_log(qd_log_source_t *log_source, qd_log_entry_t *entry)
         if (syslog_level != -1)
             syslog(syslog_level, "%s", log_str);
     }
-    sys_mutex_unlock(log_sink_list_lock);
+    sys_mutex_unlock(log_source->lock);
 }
 
 /// Reset the log source to the default state

Review comment:
       log_source->sink = 0 will drop a reference to the sink without decref'ing the sink's reference count

##########
File path: src/log.c
##########
@@ -59,12 +88,11 @@ struct qd_log_entry_t {
     struct timeval  time;
     char            text[TEXT_MAX];
 };
-
 ALLOC_DECLARE(qd_log_entry_t);
 ALLOC_DEFINE(qd_log_entry_t);
-
 DEQ_DECLARE(qd_log_entry_t, qd_log_list_t);
 static qd_log_list_t         entries = {0};
+sys_mutex_t * entries_lock = 0;

Review comment:
       code convention: please remove space between * and entries_lock

##########
File path: src/log.c
##########
@@ -506,23 +504,49 @@ PyObject *qd_log_recent_py(long limit) {
     return NULL;
 }
 
+static void _add_log_source (const char * module_name) {
+    qd_log_source_t *log_source;
+    log_source = NEW(qd_log_source_t);
+    ZERO(log_source);
+    log_source->module = (char*) malloc(strlen(module_name) + 1);

Review comment:
       pro-tip: replace lines 511+512 with a single call to qd_strdup (see ctools.h).

##########
File path: src/log.c
##########
@@ -365,41 +390,21 @@ static void qd_log_source_defaults(qd_log_source_t *log_source) {
     memset ( log_source->severity_histogram, 0, sizeof(uint64_t) * (N_LEVEL_INDICES) );
 }
 
-/// Caller must hold the log_source_lock
-static qd_log_source_t *qd_log_source_lh(const char *module)
-{
-    qd_log_source_t *log_source = lookup_log_source_lh(module);
-    if (!log_source)
-    {
-        log_source = NEW(qd_log_source_t);
-        ZERO(log_source);
-        log_source->module = (char*) malloc(strlen(module) + 1);
-        strcpy(log_source->module, module);
-        qd_log_source_defaults(log_source);
-        DEQ_INSERT_TAIL(source_list, log_source);
-        qd_entity_cache_add(QD_LOG_STATS_TYPE, log_source);
-    }
-    return log_source;
-}
-
 qd_log_source_t *qd_log_source(const char *module)
 {
-    sys_mutex_lock(log_source_lock);
-    qd_log_source_t* src = qd_log_source_lh(module);
-    sys_mutex_unlock(log_source_lock);
+    qd_log_source_t* src = lookup_log_source(module);
     return src;
 }
 
 qd_log_source_t *qd_log_source_reset(const char *module)
 {
-    sys_mutex_lock(log_source_lock);
-    qd_log_source_t* src = qd_log_source_lh(module);
+    qd_log_source_t* src = qd_log_source(module);
     qd_log_source_defaults(src);
-    sys_mutex_unlock(log_source_lock);
     return src;
 }
 
-static void qd_log_source_free_lh(qd_log_source_t* src) {
+// This is called only during finalize, which does not hold locks.
+static void qd_log_source_free(qd_log_source_t* src) {

Review comment:
       Need to free src->lock as well




-- 
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.

To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org

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