1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100
|
From: Samuel Thibault <samuel.thibault@ens-lyon.org>
Subject: [PATCH] The critical section lock _can_ be held in these place.
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 <samuel.thibault@ens-lyon.org>
* 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.
---
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 0ced7f3..1de90c0 100644
--- a/hurd/hurdexec.c
+++ b/hurd/hurdexec.c
@@ -104,7 +104,6 @@ _hurd_exec (task_t task, file_t file,
ss = _hurd_self_sigstate ();
- assert (! __spin_lock_locked (&ss->critical_section_lock));
__spin_lock (&ss->critical_section_lock);
__spin_lock (&ss->lock);
diff --git a/hurd/thread-cancel.c b/hurd/thread-cancel.c
index c7f88ee..eaf3d9e 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 bdc24b9..0422d9e 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 867579d..bcd06dd 100644
--- a/sysdeps/mach/hurd/spawni.c
+++ b/sysdeps/mach/hurd/spawni.c
@@ -235,7 +235,6 @@ __spawni (pid_t *pid, const char *file,
ss = _hurd_self_sigstate ();
- assert (! __spin_lock_locked (&ss->critical_section_lock));
__spin_lock (&ss->critical_section_lock);
__spin_lock (&ss->lock);
--
tg: (9a079e2..) t/thread-cancel (depends on: baseline)
|