-
Notifications
You must be signed in to change notification settings - Fork 298
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Properly merge header fields #14
Conversation
In `index_mail_add_temp_wanted_fields' properly merge header fields by dropping duplicates rather than appending them over and over again. Time complexity could be improved as this now performs in O(n^2) for normally small n, though. Credits to Matthias Schaff <matthias.schaff@inovex.de>
I was assuming that mailbox_header_lookup_init() did the deduplication, but looking at its code I don't think it does. So this deduplication could be moved there and it could help with other parts of the code as well. Also it already sorts the headers so it would also avoid the O(n^2). |
Timo, thanks for your feedback. I've discussed with my colleague who created this patch and we came to the conclusion that this patch isn't necessary and the issue behind it needs a solution on our side (instead on Dovecot side). Closing this PR as described above.. |
This fixes a race condition where the http_client_host_shared_idle_timeout() function would get called with an already freed hshared argument. Specifically, the situation arises from the hshared idle timeout calling http_client_host_shared_free(), which removes the timeout and then proceeds to free the client queue. The client queue freeing code indirectly calls http_client_host_shared_check_idle(), which notices that there is no idle timeout and allocates one. The backtrace at the point of this new timeout allocation: frame #3: 0x00007f0c775897f0 libdovecot.so.0`timeout_add_to(...) ioloop.c:280 frame #4: 0x00007f0c7751a45f libdovecot.so.0`http_client_host_shared_check_idle(hshared=<unavailable>) at http-client-host.c:69 frame #5: 0x00007f0c7750de89 libdovecot.so.0`http_client_request_error(_req=<unavailable>, status=9000, error="") at http-client-request.c:1525 frame #6: 0x00007f0c77517f38 libdovecot.so.0`http_client_queue_fail_full(queue=0x000055e13cff0e10, status=9000, error="", all=<unavailable>) at http-client-queue.c:183 frame #7: 0x00007f0c77518baa libdovecot.so.0`http_client_queue_free(queue=0x000055e13cff0e10) at http-client-queue.c:141 frame #8: 0x00007f0c7751a8bc libdovecot.so.0`http_client_host_free_shared(_host=<unavailable>) at http-client-host.c:391 frame #9: 0x00007f0c7751ab4c libdovecot.so.0`http_client_host_shared_free(_hshared=0x00007ffdac109e48) at http-client-host.c:294 frame #10: 0x00007f0c7751ace8 libdovecot.so.0`http_client_host_shared_idle_timeout(hshared=<unavailable>) at http-client-host.c:40 frame #11: 0x00007f0c7758a1a4 libdovecot.so.0`io_loop_handle_timeouts at ioloop.c:682 frame #12: 0x00007f0c7758a089 libdovecot.so.0`io_loop_handle_timeouts(ioloop=0x000055e13cfc8d80) at ioloop.c:696 frame #13: 0x00007f0c7758befc libdovecot.so.0`io_loop_handler_run_internal(ioloop=0x000055e13cfc8d80) at ioloop-select.c:126 frame #14: 0x00007f0c7758a56d libdovecot.so.0`io_loop_handler_run(ioloop=<unavailable>) at ioloop.c:767 frame #15: 0x00007f0c7758a798 libdovecot.so.0`io_loop_run(ioloop=0x000055e13cfc8d80) at ioloop.c:740 frame #16: 0x00007f0c774f61eb libdovecot.so.0`master_service_run(service=0x000055e13cfc8c10, callback=<unavailable>) at master-service.c:782 frame #17: 0x000055e13b48e3a5 stats`main(argc=<unavailable>, argv=<unavailable>) at main.c:99 frame #18: 0x00007f0c771092e1 libc.so.6`__libc_start_main + 241 frame #19: 0x000055e13b48e41a stats`_start + 42
This fixes a race condition where the http_client_host_shared_idle_timeout() function would get called with an already freed hshared argument. Specifically, the situation arises from the hshared idle timeout calling http_client_host_shared_free(), which removes the timeout and then proceeds to free the client queue. The client queue freeing code indirectly calls http_client_host_shared_check_idle(), which notices that there is no idle timeout and allocates one. The backtrace at the point of this new timeout allocation: frame #3: 0x00007f0c775897f0 libdovecot.so.0`timeout_add_to(...) ioloop.c:280 frame #4: 0x00007f0c7751a45f libdovecot.so.0`http_client_host_shared_check_idle(hshared=<unavailable>) at http-client-host.c:69 frame #5: 0x00007f0c7750de89 libdovecot.so.0`http_client_request_error(_req=<unavailable>, status=9000, error="") at http-client-request.c:1525 frame #6: 0x00007f0c77517f38 libdovecot.so.0`http_client_queue_fail_full(queue=0x000055e13cff0e10, status=9000, error="", all=<unavailable>) at http-client-queue.c:183 frame #7: 0x00007f0c77518baa libdovecot.so.0`http_client_queue_free(queue=0x000055e13cff0e10) at http-client-queue.c:141 frame #8: 0x00007f0c7751a8bc libdovecot.so.0`http_client_host_free_shared(_host=<unavailable>) at http-client-host.c:391 frame #9: 0x00007f0c7751ab4c libdovecot.so.0`http_client_host_shared_free(_hshared=0x00007ffdac109e48) at http-client-host.c:294 frame #10: 0x00007f0c7751ace8 libdovecot.so.0`http_client_host_shared_idle_timeout(hshared=<unavailable>) at http-client-host.c:40 frame #11: 0x00007f0c7758a1a4 libdovecot.so.0`io_loop_handle_timeouts at ioloop.c:682 frame #12: 0x00007f0c7758a089 libdovecot.so.0`io_loop_handle_timeouts(ioloop=0x000055e13cfc8d80) at ioloop.c:696 frame #13: 0x00007f0c7758befc libdovecot.so.0`io_loop_handler_run_internal(ioloop=0x000055e13cfc8d80) at ioloop-select.c:126 frame #14: 0x00007f0c7758a56d libdovecot.so.0`io_loop_handler_run(ioloop=<unavailable>) at ioloop.c:767 frame #15: 0x00007f0c7758a798 libdovecot.so.0`io_loop_run(ioloop=0x000055e13cfc8d80) at ioloop.c:740 frame #16: 0x00007f0c774f61eb libdovecot.so.0`master_service_run(service=0x000055e13cfc8c10, callback=<unavailable>) at master-service.c:782 frame #17: 0x000055e13b48e3a5 stats`main(argc=<unavailable>, argv=<unavailable>) at main.c:99 frame #18: 0x00007f0c771092e1 libc.so.6`__libc_start_main + 241 frame #19: 0x000055e13b48e41a stats`_start + 42
This fixes a race condition where the http_client_host_shared_idle_timeout() function would get called with an already freed hshared argument. Specifically, the situation arises from the hshared idle timeout calling http_client_host_shared_free(), which removes the timeout and then proceeds to free the client queue. The client queue freeing code indirectly calls http_client_host_shared_check_idle(), which notices that there is no idle timeout and allocates one. The backtrace at the point of this new timeout allocation: frame #3: 0x00007f0c775897f0 libdovecot.so.0`timeout_add_to(...) ioloop.c:280 frame #4: 0x00007f0c7751a45f libdovecot.so.0`http_client_host_shared_check_idle(hshared=<unavailable>) at http-client-host.c:69 frame #5: 0x00007f0c7750de89 libdovecot.so.0`http_client_request_error(_req=<unavailable>, status=9000, error="") at http-client-request.c:1525 frame #6: 0x00007f0c77517f38 libdovecot.so.0`http_client_queue_fail_full(queue=0x000055e13cff0e10, status=9000, error="", all=<unavailable>) at http-client-queue.c:183 frame #7: 0x00007f0c77518baa libdovecot.so.0`http_client_queue_free(queue=0x000055e13cff0e10) at http-client-queue.c:141 frame #8: 0x00007f0c7751a8bc libdovecot.so.0`http_client_host_free_shared(_host=<unavailable>) at http-client-host.c:391 frame #9: 0x00007f0c7751ab4c libdovecot.so.0`http_client_host_shared_free(_hshared=0x00007ffdac109e48) at http-client-host.c:294 frame #10: 0x00007f0c7751ace8 libdovecot.so.0`http_client_host_shared_idle_timeout(hshared=<unavailable>) at http-client-host.c:40 frame #11: 0x00007f0c7758a1a4 libdovecot.so.0`io_loop_handle_timeouts at ioloop.c:682 frame #12: 0x00007f0c7758a089 libdovecot.so.0`io_loop_handle_timeouts(ioloop=0x000055e13cfc8d80) at ioloop.c:696 frame #13: 0x00007f0c7758befc libdovecot.so.0`io_loop_handler_run_internal(ioloop=0x000055e13cfc8d80) at ioloop-select.c:126 frame #14: 0x00007f0c7758a56d libdovecot.so.0`io_loop_handler_run(ioloop=<unavailable>) at ioloop.c:767 frame #15: 0x00007f0c7758a798 libdovecot.so.0`io_loop_run(ioloop=0x000055e13cfc8d80) at ioloop.c:740 frame #16: 0x00007f0c774f61eb libdovecot.so.0`master_service_run(service=0x000055e13cfc8c10, callback=<unavailable>) at master-service.c:782 frame #17: 0x000055e13b48e3a5 stats`main(argc=<unavailable>, argv=<unavailable>) at main.c:99 frame #18: 0x00007f0c771092e1 libc.so.6`__libc_start_main + 241 frame #19: 0x000055e13b48e41a stats`_start + 42
In `index_mail_add_temp_wanted_fields' properly merge header fields by dropping duplicates rather than appending them over and over again.
Time complexity could be improved as this now performs in O(n^2) for normally small n, though.
Credits to Matthias Schaff matthias.schaff@inovex.de