iothread: Make iothread_stop() idempotent

[Qemu-devel] [PATCH] iothread: Make iothread_stop() idempotent

Posted by Eduardo Habkost 8 years, 4 months ago

Currently, iothread_stop_all() makes all iothread objects unsafe
to be destroyed, because qemu_thread_join() ends up being called
twice.

To fix this, make iothread_stop() idempotent by checking
thread->stopped.

Fixes the following crash:

  qemu-system-x86_64 -object iothread,id=iothread0 -monitor stdio -display none
  QEMU 2.10.50 monitor - type 'help' for more information
  (qemu) quit
  qemu: qemu_thread_join: No such process
  Aborted (core dumped)

Reported-by: Christian Borntraeger <borntraeger@de.ibm.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 iothread.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/iothread.c b/iothread.c
index 44c8944dc4..59d0850988 100644
--- a/iothread.c
+++ b/iothread.c
@@ -85,7 +85,7 @@ static int iothread_stop(Object *object, void *opaque)
     IOThread *iothread;
 
     iothread = (IOThread *)object_dynamic_cast(object, TYPE_IOTHREAD);
-    if (!iothread || !iothread->ctx) {
+    if (!iothread || !iothread->ctx || iothread->stopping) {
         return 0;
     }
     iothread->stopping = true;
-- 
2.13.5

Re: [Qemu-devel] [PATCH] iothread: Make iothread_stop() idempotent

Posted by Christian Borntraeger 8 years, 4 months ago


On 09/26/2017 03:00 PM, Eduardo Habkost wrote:
> Currently, iothread_stop_all() makes all iothread objects unsafe
> to be destroyed, because qemu_thread_join() ends up being called
> twice.
> 
> To fix this, make iothread_stop() idempotent by checking
> thread->stopped.
> 
> Fixes the following crash:
> 
>   qemu-system-x86_64 -object iothread,id=iothread0 -monitor stdio -display none
>   QEMU 2.10.50 monitor - type 'help' for more information
>   (qemu) quit
>   qemu: qemu_thread_join: No such process
>   Aborted (core dumped)
> 
> Reported-by: Christian Borntraeger <borntraeger@de.ibm.com>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>

Tested-by: Christian Borntraeger <borntraeger@de.ibm.com>

> ---
>  iothread.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/iothread.c b/iothread.c
> index 44c8944dc4..59d0850988 100644
> --- a/iothread.c
> +++ b/iothread.c
> @@ -85,7 +85,7 @@ static int iothread_stop(Object *object, void *opaque)
>      IOThread *iothread;
> 
>      iothread = (IOThread *)object_dynamic_cast(object, TYPE_IOTHREAD);
> -    if (!iothread || !iothread->ctx) {
> +    if (!iothread || !iothread->ctx || iothread->stopping) {
>          return 0;
>      }
>      iothread->stopping = true;
>

Re: [Qemu-devel] [PATCH] iothread: Make iothread_stop() idempotent

Posted by Christian Borntraeger 8 years, 4 months ago

Is anybody going to pick this up? upstream qemu is still happily filling
up my disk with coredumps on exit.

On 09/26/2017 03:00 PM, Eduardo Habkost wrote:
> Currently, iothread_stop_all() makes all iothread objects unsafe
> to be destroyed, because qemu_thread_join() ends up being called
> twice.
> 
> To fix this, make iothread_stop() idempotent by checking
> thread->stopped.
> 
> Fixes the following crash:
> 
>   qemu-system-x86_64 -object iothread,id=iothread0 -monitor stdio -display none
>   QEMU 2.10.50 monitor - type 'help' for more information
>   (qemu) quit
>   qemu: qemu_thread_join: No such process
>   Aborted (core dumped)
> 
> Reported-by: Christian Borntraeger <borntraeger@de.ibm.com>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
>  iothread.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/iothread.c b/iothread.c
> index 44c8944dc4..59d0850988 100644
> --- a/iothread.c
> +++ b/iothread.c
> @@ -85,7 +85,7 @@ static int iothread_stop(Object *object, void *opaque)
>      IOThread *iothread;
> 
>      iothread = (IOThread *)object_dynamic_cast(object, TYPE_IOTHREAD);
> -    if (!iothread || !iothread->ctx) {
> +    if (!iothread || !iothread->ctx || iothread->stopping) {
>          return 0;
>      }
>      iothread->stopping = true;
>

Re: [Qemu-devel] [PATCH] iothread: Make iothread_stop() idempotent

Posted by Paolo Bonzini 8 years, 4 months ago

On 29/09/2017 15:47, Christian Borntraeger wrote:
> Is anybody going to pick this up? upstream qemu is still happily filling
> up my disk with coredumps on exit.

I can, but I'll only send the pull request next Monday, probably.

Paolo

> On 09/26/2017 03:00 PM, Eduardo Habkost wrote:
>> Currently, iothread_stop_all() makes all iothread objects unsafe
>> to be destroyed, because qemu_thread_join() ends up being called
>> twice.
>>
>> To fix this, make iothread_stop() idempotent by checking
>> thread->stopped.
>>
>> Fixes the following crash:
>>
>>   qemu-system-x86_64 -object iothread,id=iothread0 -monitor stdio -display none
>>   QEMU 2.10.50 monitor - type 'help' for more information
>>   (qemu) quit
>>   qemu: qemu_thread_join: No such process
>>   Aborted (core dumped)
>>
>> Reported-by: Christian Borntraeger <borntraeger@de.ibm.com>
>> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
>> ---
>>  iothread.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/iothread.c b/iothread.c
>> index 44c8944dc4..59d0850988 100644
>> --- a/iothread.c
>> +++ b/iothread.c
>> @@ -85,7 +85,7 @@ static int iothread_stop(Object *object, void *opaque)
>>      IOThread *iothread;
>>
>>      iothread = (IOThread *)object_dynamic_cast(object, TYPE_IOTHREAD);
>> -    if (!iothread || !iothread->ctx) {
>> +    if (!iothread || !iothread->ctx || iothread->stopping) {
>>          return 0;
>>      }
>>      iothread->stopping = true;
>>
>

Re: [Qemu-devel] [PATCH] iothread: Make iothread_stop() idempotent

Posted by Peter Maydell 8 years, 4 months ago

On 29 September 2017 at 07:13, Paolo Bonzini <pbonzini@redhat.com> wrote:
> On 29/09/2017 15:47, Christian Borntraeger wrote:
>> Is anybody going to pick this up? upstream qemu is still happily filling
>> up my disk with coredumps on exit.
>
> I can, but I'll only send the pull request next Monday, probably.

I'm not likely to be able to apply any pull requests til Monday anyway :-)

thanks
-- PMM