[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [RFC PATCH glibc 25/34] hurd: Improve reply port handling when exiti
From: |
Samuel Thibault |
Subject: |
Re: [RFC PATCH glibc 25/34] hurd: Improve reply port handling when exiting signal handlers |
Date: |
Tue, 11 Apr 2023 00:03:11 +0200 |
User-agent: |
NeoMutt/20170609 (1.8.3) |
Applied, thanks!
Sergey Bugaev, le dim. 19 mars 2023 18:10:08 +0300, a ecrit:
> NOTE: I don't really understand why sigunwind wants to destroy both its
> current reply port and user's reply port. Prior to commit
> fb304035c41c7ee2afede51e5e8568974549ba5e, it was *restoring* the user's
> reply port, in which case it actually made sense to destroy the current
> reply port. Post-fb304035c41c7ee2afede51e5e8568974549ba5e, wouldn't it
> be better to just keep using the current reply port, destroying the
> user's one?
We could try that, yes.
I tend to be very cautious with reply port reuse since it can confuse
servers a lot when e.g. interrupted, so it's generally safer not to try
to reuse them.
> hurd/sigunwind.c | 24 +++++++++++-------------
> sysdeps/mach/hurd/i386/sigreturn.c | 21 +++++----------------
> 2 files changed, 16 insertions(+), 29 deletions(-)
>
> diff --git a/hurd/sigunwind.c b/hurd/sigunwind.c
> index edd40b14..399e6900 100644
> --- a/hurd/sigunwind.c
> +++ b/hurd/sigunwind.c
> @@ -18,7 +18,6 @@
>
> #include <hurd.h>
> #include <thread_state.h>
> -#include <hurd/threadvar.h>
> #include <jmpbuf-unwind.h>
> #include <assert.h>
> #include <stdint.h>
> @@ -39,19 +38,18 @@ _hurdsig_longjmp_from_handler (void *data, jmp_buf env,
> int val)
> {
> /* Destroy the MiG reply port used by the signal handler, and restore
> the reply port in use by the thread when interrupted. */
> - mach_port_t *reply_port = &__hurd_local_reply_port;
> - if (*reply_port)
> - {
> - mach_port_t port = *reply_port;
> - /* Assigning MACH_PORT_DEAD here tells libc's mig_get_reply_port
> - not to get another reply port, but avoids mig_dealloc_reply_port
> - trying to deallocate it after the receive fails (which it will,
> - because the reply port will be bogus, regardless). */
> - *reply_port = MACH_PORT_DEAD;
> - __mach_port_destroy (__mach_task_self (), port);
> - }
> + mach_port_t reply_port = THREAD_GETMEM (THREAD_SELF, reply_port);
> + /* Assigning MACH_PORT_DEAD here tells libc's mig_get_reply_port not to
> + get another reply port, but avoids mig_dealloc_reply_port trying to
> + deallocate it after the receive fails (which it will, because the
> + reply port will be bogus, regardless). */
> + THREAD_SETMEM (THREAD_SELF, reply_port, MACH_PORT_DEAD);
> + if (MACH_PORT_VALID (reply_port))
> + __mach_port_mod_refs (__mach_task_self (), reply_port,
> + MACH_PORT_RIGHT_RECEIVE, -1);
> if (scp->sc_reply_port)
> - __mach_port_destroy (__mach_task_self (), scp->sc_reply_port);
> + __mach_port_mod_refs (__mach_task_self (), scp->sc_reply_port,
> + MACH_PORT_RIGHT_RECEIVE, -1);
> }
>
> __spin_lock (&ss->lock);
> diff --git a/sysdeps/mach/hurd/i386/sigreturn.c
> b/sysdeps/mach/hurd/i386/sigreturn.c
> index db1a01f3..29c9629f 100644
> --- a/sysdeps/mach/hurd/i386/sigreturn.c
> +++ b/sysdeps/mach/hurd/i386/sigreturn.c
> @@ -19,7 +19,6 @@ register int *sp asm ("%esp");
>
> #include <hurd.h>
> #include <hurd/signal.h>
> -#include <hurd/threadvar.h>
> #include <hurd/msg.h>
> #include <stdlib.h>
> #include <string.h>
> @@ -59,7 +58,7 @@ __sigreturn (struct sigcontext *scp)
> {
> struct hurd_sigstate *ss;
> struct hurd_userlink *link = (void *) &scp[1];
> - mach_port_t *reply_port;
> + mach_port_t reply_port;
>
> if (scp == NULL || (scp->sc_mask & _SIG_CANT_MASK))
> {
> @@ -101,20 +100,10 @@ __sigreturn (struct sigcontext *scp)
>
> /* Destroy the MiG reply port used by the signal handler, and restore the
> reply port in use by the thread when interrupted. */
> - reply_port = &__hurd_local_reply_port;
> - if (*reply_port)
> - {
> - mach_port_t port = *reply_port;
> -
> - /* Assigning MACH_PORT_DEAD here tells libc's mig_get_reply_port not to
> - get another reply port, but avoids mig_dealloc_reply_port trying to
> - deallocate it after the receive fails (which it will, because the
> - reply port will be bogus, whether we do this or not). */
> - *reply_port = MACH_PORT_DEAD;
> -
> - __mach_port_destroy (__mach_task_self (), port);
> - }
> - *reply_port = scp->sc_reply_port;
> + reply_port = THREAD_GETMEM (THREAD_SELF, reply_port);
> + THREAD_SETMEM (THREAD_SELF, reply_port, scp->sc_reply_port);
> + __mach_port_mod_refs (__mach_task_self (), reply_port,
> + MACH_PORT_RIGHT_RECEIVE, -1);
>
> if (scp->sc_fpused)
> /* Restore the FPU state. Mach conveniently stores the state
> --
> 2.39.2
>
--
Samuel
---
Pour une évaluation indépendante, transparente et rigoureuse !
Je soutiens la Commission d'Évaluation de l'Inria.
- Re: [RFC PATCH glibc 25/34] hurd: Improve reply port handling when exiting signal handlers,
Samuel Thibault <=