qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH] tests: Disable test-bdrv-drain


From: Eric Blake
Subject: Re: [Qemu-devel] [PATCH] tests: Disable test-bdrv-drain
Date: Mon, 8 Oct 2018 14:53:32 -0500
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.0

On 10/8/18 11:40 AM, Kevin Wolf wrote:
Am 08.10.2018 um 17:43 hat Peter Maydell geschrieben:
Looking at the backtraces I'm wondering if this is the result of
an implicit reliance on the order in which per-thread destructors
are called (which is left unspecified by POSIX) -- the destructor
function qemu_thread_atexit_run() is called after some other
destructor, but accesses its memory.

Specifically, the memory it's trying to read looks like
the __thread local variable pollfds_cleanup_notifier in
util/aio-posix.c. So I think what is happening is:
  * util/aio-posix.c calls qemu_thread_atexit_add(), passing
    it a pointer to a thread-local variable pollfds_cleanup_notifier
  * qemu_thread_atexit_add() works by arranging to run the
    notifiers when its 'exit_key' variable's destructor is called
  * the destructor for pollfds_cleanup_notifier runs before that
    for exit_key, and so the qemu_thread_atexit_run() function
    ends up touching freed memory

I'm pretty confident this analysis of the problem is correct:
unfortunately I have no idea what the right way to fix it is...

Yes, I agree with your analysis. If __thread variables can be destructed
before pthread_key_create() destructors are called (and in particular if
the former are implemented in terms of the latter), this implies at
least two rules:

1. The Notfier itself can't be a TLS variable

2. The notifier callback can't access any TLS variables

Of course, with these restrictions, qemu_thread_atexit_*() with its
existing API is as useless as it could be.

The best I can think of at the moment would be to use a separate
pthread_key_create() (and therefore a separate destructor) for
registering each TLS variable, so that the destructor always gets a
valid pointer. Maybe move all __thread variables of a file into a single
malloced struct to make it more managable (we could then keep a __thread
pointer to it for convenience, but only free the struct with the pointer
passed by the pthread_key destructor so that we don't have to access
__thread variables in the destructor).

pthread_key_create() says that a when a destructor is triggered, it sets the value of the key to NULL; but that you can once again set the key back to a non-NULL value, and that the implementation will loop at least PTHREAD_DESTRUCTOR_ITERATIONS over all destructors or until it has convinced the destructors to leave values at NULL. Thus, while you cannot guarantee ordering between destructors within a single iteration of the cleanup loop, you CAN do some sort of witness locking or down-counter where a destructor purposefully calls pthread_setspecific() to revive the value to survive into the next iteration of destructor calls, for variables which are known to be referenced by other destructors while the witness count is still high enough, as a way of imposing order between loops.

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



reply via email to

[Prev in Thread] Current Thread [Next in Thread]