Patchwork [2/3] qemu-thread: add cleanup_push() and cleanup_pop()

login
register
mail settings
Submitter Corentin Chary
Date May 29, 2010, 7:38 a.m.
Message ID <1275118686-15649-3-git-send-email-corentincj@iksaif.net>
Download mbox | patch
Permalink /patch/53980/
State New
Headers show

Comments

Corentin Chary - May 29, 2010, 7:38 a.m.
From pthread man:

 These  functions  manipulate  the  calling thread's stack of
 thread-cancellation clean-up handlers.  A clean-up handler is
 a function that is automatically executed when a thread is canceled
 [...] it might, for example, unlock a mutex so that it becomes
 available to other threads in the process.

These two functions are implemented using macros because there is no
other way to do that (pthread man, again):

 On Linux, the pthread_cleanup_push() and pthread_cleanup_pop()
 functions are implemented as macros that expand to text  containing
 '{'  and  '}',  respectively.  This means that variables declared
 within the scope of paired calls to these functions will only
 be visible within that scope.

Signed-off-by: Corentin Chary <corentincj@iksaif.net>
---
 qemu-thread.h |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)
Paul Brook - June 3, 2010, 5:50 a.m.
> From pthread man:
> 
>  These  functions  manipulate  the  calling thread's stack of
>  thread-cancellation clean-up handlers.  A clean-up handler is
>  a function that is automatically executed when a thread is canceled
>  [...] it might, for example, unlock a mutex so that it becomes
>  available to other threads in the process.

Do we really need to use thread cancellation?
It's one of those features that makes me extremely nervous. Especially in C 
code where people generally aren't expecting exceptions to be thrown.

Paul
Paolo Bonzini - June 3, 2010, 7:27 a.m.
On 05/29/2010 09:38 AM, Corentin Chary wrote:
> Signed-off-by: Corentin Chary<corentincj@iksaif.net>
> ---
>   qemu-thread.h |    4 ++++
>   1 files changed, 4 insertions(+), 0 deletions(-)
>
> diff --git a/qemu-thread.h b/qemu-thread.h
> index 19bb30c..e5006bb 100644
> --- a/qemu-thread.h
> +++ b/qemu-thread.h
> @@ -41,4 +41,8 @@ void qemu_thread_self(QemuThread *thread);
>   int qemu_thread_equal(QemuThread *thread1, QemuThread *thread2);
>   void qemu_thread_exit(void *retval);
>
> +#define qemu_thread_cleanup_pop(execute) pthread_cleanup_pop(execute)
> +#define qemu_thread_cleanup_push(routine, arg)  \
> +    pthread_cleanup_push(routine, arg)

I agree with Paul that this isn't necessary.  Also you're not using 
pthread_exit.  Probably stale from a previous version of the patch?

Paolo
Corentin Chary - June 3, 2010, 7:46 a.m.
On Thu, Jun 3, 2010 at 9:27 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> On 05/29/2010 09:38 AM, Corentin Chary wrote:
>>
>> Signed-off-by: Corentin Chary<corentincj@iksaif.net>
>> ---
>>  qemu-thread.h |    4 ++++
>>  1 files changed, 4 insertions(+), 0 deletions(-)
>>
>> diff --git a/qemu-thread.h b/qemu-thread.h
>> index 19bb30c..e5006bb 100644
>> --- a/qemu-thread.h
>> +++ b/qemu-thread.h
>> @@ -41,4 +41,8 @@ void qemu_thread_self(QemuThread *thread);
>>  int qemu_thread_equal(QemuThread *thread1, QemuThread *thread2);
>>  void qemu_thread_exit(void *retval);
>>
>> +#define qemu_thread_cleanup_pop(execute) pthread_cleanup_pop(execute)
>> +#define qemu_thread_cleanup_push(routine, arg)  \
>> +    pthread_cleanup_push(routine, arg)
>
> I agree with Paul that this isn't necessary.  Also you're not using
> pthread_exit.  Probably stale from a previous version of the patch?

Right pthread_exit() is missing.

Anyway, I don't use thread cancellation in the threaded vnc server (I
send a signal to the condition with an empty queue to stop the
thread), so we can skip this patch.

Patch

diff --git a/qemu-thread.h b/qemu-thread.h
index 19bb30c..e5006bb 100644
--- a/qemu-thread.h
+++ b/qemu-thread.h
@@ -41,4 +41,8 @@  void qemu_thread_self(QemuThread *thread);
 int qemu_thread_equal(QemuThread *thread1, QemuThread *thread2);
 void qemu_thread_exit(void *retval);
 
+#define qemu_thread_cleanup_pop(execute) pthread_cleanup_pop(execute)
+#define qemu_thread_cleanup_push(routine, arg)  \
+    pthread_cleanup_push(routine, arg)
+
 #endif