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

Hi Monty, On Fri, May 06, 2016 at 12:52:02PM +0300, Michael Widenius wrote:

Hi!

On 5 May 2016 17:42, "Sergey Vojtovich" <svoj@mariadb.org> wrote:

...

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.

Sorry, I missed the part about service threads. For these I don't mind to use joinable threads. Ok, mystery solved now! Thanks for confirming you're not against them in general.

Just one question: How to handle hanged threads? With the old code we could, if this would ever be a problem, just change cond_wait to cond_timed_wait. Don't know how to do this with joinable threads, except using alarms which is a bit of code (20 lines)

I don't think pthread_join() is good for hanged threads. But threads should never hang either. Regards, Sergey