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 :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list