Bug 790837

Summary: Use of atexit to clean up handles is wrong in multithreaded programs
Product: [Community] Virtualization Tools Reporter: Richard W.M. Jones <rjones>
Component: libguestfsAssignee: Richard W.M. Jones <rjones>
Status: NEW --- QA Contact:
Severity: unspecified Docs Contact:
Priority: unspecified    
Version: unspecifiedCC: virt-maint
Target Milestone: ---   
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:

Description Richard W.M. Jones 2012-02-15 14:13:50 UTC
Description of problem:

libguestfs registers an atexit handler to clean up handles.
https://github.com/libguestfs/libguestfs/blob/84a4160fd30c46575b93f87f6ffc7cc556b0af93/src/guestfs.c#L149
This fails badly in multithreaded programs.  If any thread calls
exit(3) then the atexit handler runs in that thread.  This can
result in a segfault in (at least) two different ways:

(i) The other threads are still running while the atexit handler
is called, so if they have any libguestfs handles themselves and
are using them, these get abruptly freed from under them.

(ii) If two threads call exit simultaneously, the atexit handler
will run twice.  POSIX notes (see exit(3p)) that programs that
call exit more than once can result in undefined behaviour.  In
the case of libguestfs this will certainly result in a double-
free.

Version-Release number of selected component (if applicable):

1.17.7

How reproducible:

Very easily with a multithreaded test program that I have.

Comment 1 Richard W.M. Jones 2012-02-15 19:42:15 UTC
I studied this problem and I cannot see a simple way to fix this
and to clean up handles when a program exits suddenly.

In order to meaningfully clean up, we need to not just kill qemu,
but also free all temporary directories in use.  The latter
essentially requires that we close the handles, but that is not
possible if threads are running and using those handles.

I also tried to use a gcc destructor instead of an atexit handler,
but these behave exactly the same way.

Comment 2 Richard W.M. Jones 2013-03-29 21:00:55 UTC
In libguestfs >= 1.21, both 'virt-df' and 'virt-alignment-scan'
are very prone to core dumping because of this bug.

The reason is that these programs are now highly multi-threaded[1].
If there is any error, they call exit(EXIT_FAILURE) which causes
close_handles() to be called, closing handles which are in use in
other threads.

[1] http://rwmj.wordpress.com/2013/02/26/new-in-libguestfs-1-21-15-parallel-virt-df-and-virt-alignment-scan/