Re: [libvirt] [PATCH] threadpool: Switch to detached threads
On Thu, May 21, 2015 at 11:57:52AM +0200, Jiri Denemark wrote:
Using joinable threads does not help anything, but it can lead to
memory
leaks.
When a worker thread exits, it decreases nWorkers or nPrioWorkers and
once both nWorkers and nPrioWorkers are zero (i.e., the last worker is
gone), quit_cond is signaled. When freeing the pool we first tell all
threads to die and then we are waiting for both nWorkers and
nPrioWorkers to become zero. At this point we already know all threads
are gone. So the only reason for calling virThreadJoin of all workers is
to free the memory allocated for joinable threads. If we avoid
allocating this memory, we don't need to take care of freeing it.
The idea behind thread join was to make debugging with valgrind better.
By waiting for the threads to shutdown we ensure that all memory they
are using has been free before libvirtd exits, so valgrind doesn't
report bogus leaks.
Another reason for using Join is that it allows the threads to finish
processing whatever RPC/API call they were currently handling. If we
daemonize all threads, then I believe that would allow libvirtd to
exit while we were in the middle of performing some change on QEMU.
This seems like it could well lead to inconsistent state where we had
updated QEMU, but not our state XML, or vica-verca.
For sake of automated restarts in RPM %post, I think allowing threads
to finish their RPC/API call is quite desirable in general.
Moreover, any memory associated with a worker thread which died
before
we asked it to die (e.g., because virCondWait failed in the thread)
would be lost anyway since virThreadPoolFree calls virThreadJoin only
for threads which were running at the time virThreadPoolFree was called.
Hmm, that is a bug - we should be calling Join on all threads we ever
created, not just those that appear to be running.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|