From 3c9fc6d7d40992ce0594428e888e76ab74e23d42 Mon Sep 17 00:00:00 2001 From: Samuel Thibault Date: Sat, 27 Aug 2022 12:38:11 +0100 Subject: [PATCH] [PATCH] The critical section lock _can_ be held in these place. MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit At least since hurd_thread_cancel can be called by another thread and lock our critical lock. http://bugs.debian.org/46859 “ Thomas suggested that there is no need to take the critical section lock. I believe that taking the critical section lock is necessary to prevent the target thread from entering a signal handler. Roland will look into the problem. ” Taking the critical section lock makes these assertions bogus. It happens that hurd_thread_cancel is only called from libports and inside /hurd/term so this is rare in practice. A reproducer can be found here: http://lists.gnu.org/archive/html/bug-hurd/2014-05/msg00025.html 2006-08-05 Samuel Thibault * hurd/thread-cancel.c (hurd_thread_cancel): Do not assert that `&ss->critical_section_lock' is unlocked. * sysdeps/mach/hurd/jmp-unwind.c (_longjmp_unwind): Likewise, and take critical section lock before taking the sigstate lock. * sysdeps/mach/hurd/spawni.c (__spawni): Likewise. Gbp-Pq: Topic hurd-i386 Gbp-Pq: Name tg-thread-cancel.diff --- hurd/hurdexec.c | 1 - hurd/thread-cancel.c | 2 -- sysdeps/mach/hurd/jmp-unwind.c | 3 +-- sysdeps/mach/hurd/spawni.c | 1 - 4 files changed, 1 insertion(+), 6 deletions(-) diff --git a/hurd/hurdexec.c b/hurd/hurdexec.c index 205afe18a..90323c6e7 100644 --- a/hurd/hurdexec.c +++ b/hurd/hurdexec.c @@ -128,7 +128,6 @@ _hurd_exec_paths (task_t task, file_t file, ss = _hurd_self_sigstate (); retry: - assert (! __spin_lock_locked (&ss->critical_section_lock)); __spin_lock (&ss->critical_section_lock); _hurd_sigstate_lock (ss); diff --git a/hurd/thread-cancel.c b/hurd/thread-cancel.c index 0ae771742..5cb8e0b1c 100644 --- a/hurd/thread-cancel.c +++ b/hurd/thread-cancel.c @@ -51,7 +51,6 @@ hurd_thread_cancel (thread_t thread) return 0; } - assert (! __spin_lock_locked (&ss->critical_section_lock)); __spin_lock (&ss->critical_section_lock); __spin_lock (&ss->lock); err = __thread_suspend (thread); @@ -91,7 +90,6 @@ hurd_check_cancel (void) int cancel; __spin_lock (&ss->lock); - assert (! __spin_lock_locked (&ss->critical_section_lock)); cancel = ss->cancel; ss->cancel = 0; __spin_unlock (&ss->lock); diff --git a/sysdeps/mach/hurd/jmp-unwind.c b/sysdeps/mach/hurd/jmp-unwind.c index 85600a9d8..bd6df8e46 100644 --- a/sysdeps/mach/hurd/jmp-unwind.c +++ b/sysdeps/mach/hurd/jmp-unwind.c @@ -49,9 +49,8 @@ _longjmp_unwind (jmp_buf env, int val) /* All access to SS->active_resources must take place inside a critical section where signal handlers cannot run. */ - __spin_lock (&ss->lock); - assert (! __spin_lock_locked (&ss->critical_section_lock)); __spin_lock (&ss->critical_section_lock); + __spin_lock (&ss->lock); /* Remove local signal preemptors being unwound past. */ while (ss->preemptors diff --git a/sysdeps/mach/hurd/spawni.c b/sysdeps/mach/hurd/spawni.c index 9941ae536..c4784e824 100644 --- a/sysdeps/mach/hurd/spawni.c +++ b/sysdeps/mach/hurd/spawni.c @@ -334,7 +334,6 @@ __spawni (pid_t *pid, const char *file, ss = _hurd_self_sigstate (); retry: - assert (! __spin_lock_locked (&ss->critical_section_lock)); __spin_lock (&ss->critical_section_lock); _hurd_sigstate_lock (ss); -- 2.30.2