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