Package: glibc / 2.28-10

hurd-i386/tg-thread-cancel.diff Patch series | download
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)