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/18 06:59:31 UTC

[GitHub] [qpid-dispatch] jiridanek opened a new pull request #1135: DISPATCH-2039 WIP Instead of poison, set zero capacity of the pool and let it degenerate into immediate malloc/free

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


   This is an alternative approach compared to #1118. From looking at the (many) errors it produces, it is just as well able to reveal issues
   
   * https://issues.apache.org/jira/browse/DISPATCH-2045 qd_hash_internal_remove_item writes to freed (pooled) memory on router shutdown
   * https://issues.apache.org/jira/browse/DISPATCH-2056 AddressSanitizer: use-after-poison in qdr_connection_set_context during system_tests_http2
   
   It is able to also find the following, that's why I had to put the no_sanitize attribute there
   
   * https://issues.apache.org/jira/browse/DISPATCH-2060 use-after free in qd_alloc_deref_safe_ptr if a pool item has been freed due to global_free_list size limit
   
   The stacktraces produced from malloc/free are much nicer than the ones from poisoning. The poisoning report is bare-bones, whereas with malloc/free
   1. there is a stacktrace for the free call
   2. the allocation stack trace is related to this particular use of the pool item, not simply the trace from when the item was first allocated if now it is being reused


-- 
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] jiridanek commented on pull request #1135: DISPATCH-2039 WIP Instead of poison, set zero capacity of the pool and let it degenerate into immediate malloc/free

Posted by GitBox <gi...@apache.org>.
jiridanek commented on pull request #1135:
URL: https://github.com/apache/qpid-dispatch/pull/1135#issuecomment-823815906


   This happens all the time on this Xenial build, https://travis-ci.com/github/apache/qpid-dispatch/jobs/499988187
   
   ```
   14: Process 12186 error: exit code -6, expected -1
   14: qdrouterd -c A.conf -I /home/travis/build/apache/qpid-dispatch/python
   14: /home/travis/build/apache/qpid-dispatch/build/tests/system_test.dir/system_tests_link_routes/EmptyTransferTest/setUpClass/A-6.cmd
   14: >>>>
   14: qdrouterd: /home/travis/build/apache/qpid-dispatch/src/alloc_pool.c:525: qd_alloc_sequence: Assertion `item->header == PATTERN_FRONT' failed.
   14: <<<<
   14: 
   14: ----------------------------------------------------------------------
   14: Ran 40 tests in 127.945s
   14: 
   14: FAILED (errors=1)
   14/72 Test #14: system_tests_link_routes ..........................***Failed  128.03 sec
   ```


-- 
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] jiridanek commented on pull request #1135: DISPATCH-2039 WIP Instead of poison, set zero capacity of the pool and let it degenerate into immediate malloc/free

Posted by GitBox <gi...@apache.org>.
jiridanek commented on pull request #1135:
URL: https://github.com/apache/qpid-dispatch/pull/1135#issuecomment-875684304


   I don't feel ready do give up on this. There are some use-after-poison jiras that might benefit from free-call stracktrace obtained by this 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] jiridanek edited a comment on pull request #1135: DISPATCH-2039 WIP Instead of poison, set zero capacity of the pool and let it degenerate into immediate malloc/free

Posted by GitBox <gi...@apache.org>.
jiridanek edited a comment on pull request #1135:
URL: https://github.com/apache/qpid-dispatch/pull/1135#issuecomment-865261894


   I'll think if I can carry this forward somewhat, but so far I don't see a way. I'll close this the next time I revisit.


-- 
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] jiridanek commented on a change in pull request #1135: DISPATCH-2039 WIP Instead of poison, set zero capacity of the pool and let it degenerate into immediate malloc/free

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



##########
File path: src/alloc_pool.c
##########
@@ -280,11 +282,12 @@ static void qd_alloc_init(qd_alloc_type_desc_t *desc)
         if (desc->additional_size)
             desc->total_size += *desc->additional_size;
 
-        if (desc->config == 0)
-            desc->config = desc->total_size > BIG_THRESHOLD ?
-                &qd_alloc_default_config_big : &qd_alloc_default_config_small;
-
-        assert (desc->config->local_free_list_max >= desc->config->transfer_batch_size);
+        desc->config = &qd_alloc_default_config_asan;
+//        if (desc->config == 0)
+//            desc->config = desc->total_size > BIG_THRESHOLD ?
+//                &qd_alloc_default_config_big : &qd_alloc_default_config_small;

Review comment:
       There will have to be a way to enable this from CMake; for now I am doing the change this way.




-- 
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] jiridanek commented on pull request #1135: DISPATCH-2039 WIP Instead of poison, set zero capacity of the pool and let it degenerate into immediate malloc/free

Posted by GitBox <gi...@apache.org>.
jiridanek commented on pull request #1135:
URL: https://github.com/apache/qpid-dispatch/pull/1135#issuecomment-865261894


   I thought if I can carry this forward somewhat, but so far I don't see a way. I'll close this the next time I revisit.


-- 
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] jiridanek commented on a change in pull request #1135: DISPATCH-2039 WIP Instead of poison, set zero capacity of the pool and let it degenerate into immediate malloc/free

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



##########
File path: src/alloc_pool.c
##########
@@ -495,7 +498,7 @@ void qd_dealloc(qd_alloc_type_desc_t *desc, qd_alloc_pool_t **tpool, char *p)
     // If there's a global_free_list size limit, remove items until the limit is
     // not exceeded.
     //
-    if (desc->config->global_free_list_max != 0) {
+    if (desc->config->global_free_list_max != -1) {

Review comment:
       0 used to mean "unlimited". I need 0 to mean 0.




-- 
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] jiridanek commented on pull request #1135: DISPATCH-2039 WIP Instead of poison, set zero capacity of the pool and let it degenerate into immediate malloc/free

Posted by GitBox <gi...@apache.org>.
jiridanek commented on pull request #1135:
URL: https://github.com/apache/qpid-dispatch/pull/1135#issuecomment-865261894


   I thought if I can carry this forward somewhat, but so far I don't see a way. I'll close this the next time I revisit.


-- 
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] jiridanek commented on a change in pull request #1135: DISPATCH-2039 WIP Instead of poison, set zero capacity of the pool and let it degenerate into immediate malloc/free

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



##########
File path: src/alloc_pool.c
##########
@@ -280,11 +282,12 @@ static void qd_alloc_init(qd_alloc_type_desc_t *desc)
         if (desc->additional_size)
             desc->total_size += *desc->additional_size;
 
-        if (desc->config == 0)
-            desc->config = desc->total_size > BIG_THRESHOLD ?
-                &qd_alloc_default_config_big : &qd_alloc_default_config_small;
-
-        assert (desc->config->local_free_list_max >= desc->config->transfer_batch_size);
+        desc->config = &qd_alloc_default_config_asan;
+//        if (desc->config == 0)
+//            desc->config = desc->total_size > BIG_THRESHOLD ?
+//                &qd_alloc_default_config_big : &qd_alloc_default_config_small;
+//
+//        assert (desc->config->local_free_list_max >= desc->config->transfer_batch_size);

Review comment:
       I think the assert is wrong; `local_free_list_max == 0` and `transfer_batch_size == 1` works just fine.




-- 
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] jiridanek closed pull request #1135: DISPATCH-2039 WIP Instead of poison, set zero capacity of the pool and let it degenerate into immediate malloc/free

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


   


-- 
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] jiridanek commented on pull request #1135: DISPATCH-2039 WIP Instead of poison, set zero capacity of the pool and let it degenerate into immediate malloc/free

Posted by GitBox <gi...@apache.org>.
jiridanek commented on pull request #1135:
URL: https://github.com/apache/qpid-dispatch/pull/1135#issuecomment-881282972


   https://issues.apache.org/jira/browse/DISPATCH-2095


-- 
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] jiridanek edited a comment on pull request #1135: DISPATCH-2039 WIP Instead of poison, set zero capacity of the pool and let it degenerate into immediate malloc/free

Posted by GitBox <gi...@apache.org>.
jiridanek edited a comment on pull request #1135:
URL: https://github.com/apache/qpid-dispatch/pull/1135#issuecomment-865261894


   I'll think if I can carry this forward somewhat, but so far I don't see a way. I'll close this the next time I revisit.


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