Hi Monty, Thanks for you reply. Please allow me to disagree with some points. :) Attached please find 2 patches, both fix this memory leak. One approach is for detached threads, another one is for joinable threads. On Thu, May 05, 2016 at 02:12:17PM +0300, Michael Widenius wrote:
Hi!
On Wed, May 4, 2016 at 11:13 AM, Sergey Vojtovich <svoj@mariadb.org> wrote:
Hi Monty,
I vaguely recall you told me once that you like detached threads more than joinable, specifically for service threads. Could you remind me what were the reasons behind it?
I need this to fix https://jira.mariadb.org/browse/MDEV-9994 properly.
The main reason is just that with joinable threads you have more work to manage the threads as you need to remember to join them as otherwise you will get a memory loss. I believe joinable.patch prooves the opposite, that is joinable threads are easier to manage.
With joinable.patch we have 32 lines less, while with detached.patch we have 4 lines more.
The only time the server has a benefit of join able threads, at least as far as I am aware of it, is at shutdown, which hasn't been a real problem as in most cases my_thread_end() will ensure that all threads has been calling my_thread_end() before the server dies.
How can my_thread_end() ensure my_thread_end() is called before server dies? I believe detached threads are only beneficial if we don't care about clean thread shutdown (that is main thread may exit any time independenly of other threads state). Otherwise we have to add code synchronizing thread shutdown like for checkpoint thread.
Referring to the valgrind issue, I would just ignore the memory loss for tls, as this isn't a real error. We are already ignoring this on other platforms.
I agree it's minor. I would agree to suppress the warning if it was generated by third party library and we could do nothing about it. But this memory leak is a consequence of API violation on MariaDB side. I'd suggest to remove all these suppressions. Even if we get warnings from other threads - it's trivial to fix. Thanks, Sergey