Merge ~bryce/ubuntu/+source/bash:ubuntu/bionic-devel into ubuntu/+source/bash:ubuntu/bionic-devel

Proposed by Bryce Harrington
Status: Merged
Approved by: Andreas Hasenack
Approved revision: cf1670027b9000c6eb6386f62701b4408886c8d5
Merge reported by: Bryce Harrington
Merged at revision: cf1670027b9000c6eb6386f62701b4408886c8d5
Proposed branch: ~bryce/ubuntu/+source/bash:ubuntu/bionic-devel
Merge into: ubuntu/+source/bash:ubuntu/bionic-devel
Diff against target: 174 lines (+152/-0)
3 files modified
debian/changelog (+7/-0)
debian/patches/bash44-020.diff (+144/-0)
debian/patches/series (+1/-0)
Reviewer Review Type Date Requested Status
Andreas Hasenack Approve
Canonical Server Core Reviewers Pending
Canonical Server Pending
Review via email: mp+368574@code.launchpad.net

Description of the change

Backport of an upstream fix to a cpu hang when waiting on background tasks.

PPA available for testing is at
https://launchpad.net/~bryce/+archive/ubuntu/bash-sru-19-010-1

I was not able to reproduce the hang myself, but apparently it can take days or weeks to reproduce. However, the fix was taken upstream and looks clearly correct to me. I'm confident given enough run time it would reproduce. It may be difficult to positively test for the bug's absence however.

I'm assuming that the target branch for this should be ubuntu/cosmic-devel; I couldn't find documentation to this effect however it seems to be what others are targeting to. Let me know if it's wrong.

I'm unclear if any tags are needed for SRU bugs but am gathering that none are. Again, let me know if otherwise.

To post a comment you must log in.
Revision history for this message
Andreas Hasenack (ahasenack) wrote :

Same comments as with the cosmic MP at https://code.launchpad.net/~bryce/ubuntu/+source/bash/+git/bash/+merge/368573.
- add dep3
- don't use -proposed

Revision history for this message
Bryce Harrington (bryce) wrote :

Updated; same changes as the other branch. Thanks again.

Revision history for this message
Andreas Hasenack (ahasenack) wrote :

+1

review: Approve
Revision history for this message
Andreas Hasenack (ahasenack) wrote :

Pushing upload tag, and then you can dput:

$ git push pkg upload/4.4.18-2ubuntu1.2
Enumerating objects: 16, done.
Counting objects: 100% (16/16), done.
Delta compression using up to 2 threads
Compressing objects: 100% (11/11), done.
Writing objects: 100% (11/11), 2.72 KiB | 558.00 KiB/s, done.
Total 11 (delta 7), reused 0 (delta 0)
To ssh://git.launchpad.net/~usd-import-team/ubuntu/+source/bash
 * [new tag] upload/4.4.18-2ubuntu1.2 -> upload/4.4.18-2ubuntu1.2

Revision history for this message
Bryce Harrington (bryce) wrote :

Thanks, pushed.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
diff --git a/debian/changelog b/debian/changelog
index 25c90d5..454a4e3 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,10 @@
1bash (4.4.18-2ubuntu1.2) bionic; urgency=medium
2
3 * d/p/bash44-020.diff: Add fix for hang on 'wait' statement
4 (LP: #1822776)
5
6 -- Bryce Harrington <bryce@canonical.com> Thu, 06 Jun 2019 15:28:15 -0700
7
1bash (4.4.18-2ubuntu1.1) bionic; urgency=medium8bash (4.4.18-2ubuntu1.1) bionic; urgency=medium
29
3 * Resurrect "Set the default path to comply with Debian policy" in10 * Resurrect "Set the default path to comply with Debian policy" in
diff --git a/debian/patches/bash44-020.diff b/debian/patches/bash44-020.diff
4new file mode 10064411new file mode 100644
index 0000000..340d085
--- /dev/null
+++ b/debian/patches/bash44-020.diff
@@ -0,0 +1,144 @@
1 BASH PATCH REPORT
2 =================
3
4Bash-Release: 4.4
5Patch-ID: bash44-020
6
7Bug-Reported-by: Graham Northup <northug@clarkson.edu>
8Bug-Reference-ID: <537530c3-61f0-349b-9de6-fa4e2487f428@clarkson.edu>
9Bug-Reference-URL: http://lists.gnu.org/archive/html/bug-bash/2017-02/msg00025.html
10
11Bug-Description:
12
13In circumstances involving long-running scripts that create and reap many
14processes, it is possible for the hash table bash uses to store exit
15statuses from asynchronous processes to develop loops. This patch fixes
16the loop causes and adds code to detect any future loops.
17
18Origin: upstream, https://ftp.gnu.org/gnu/bash/bash-4.4-patches/bash44-020
19Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/+source/bash/+bug/1822776
20Last-Update: 2019-06-10
21
22diff --git a/jobs.c b/jobs.c
23index fc96603..2684632 100644
24--- a/jobs.c
25+++ b/jobs.c
26@@ -812,8 +812,22 @@ bgp_add (pid, status)
27 ps_index_t *bucket, psi;
28 struct pidstat *ps;
29
30- bucket = pshash_getbucket (pid);
31- psi = bgp_getindex ();
32+ /* bucket == existing chain of pids hashing to same value
33+ psi = where were going to put this pid/status */
34+
35+ bucket = pshash_getbucket (pid); /* index into pidstat_table */
36+ psi = bgp_getindex (); /* bgpids.head, index into storage */
37+
38+ /* XXX - what if psi == *bucket? */
39+ if (psi == *bucket)
40+ {
41+#ifdef DEBUG
42+ internal_warning ("hashed pid %d (pid %d) collides with bgpids.head, skipping", psi, pid);
43+#endif
44+ bgpids.storage[psi].pid = NO_PID; /* make sure */
45+ psi = bgp_getindex (); /* skip to next one */
46+ }
47+
48 ps = &bgpids.storage[psi];
49
50 ps->pid = pid;
51@@ -841,32 +855,47 @@ pshash_delindex (psi)
52 ps_index_t psi;
53 {
54 struct pidstat *ps;
55+ ps_index_t *bucket;
56
57 ps = &bgpids.storage[psi];
58 if (ps->pid == NO_PID)
59 return;
60
61- if (ps->bucket_next != NO_PID)
62+ if (ps->bucket_next != NO_PIDSTAT)
63 bgpids.storage[ps->bucket_next].bucket_prev = ps->bucket_prev;
64- if (ps->bucket_prev != NO_PID)
65+ if (ps->bucket_prev != NO_PIDSTAT)
66 bgpids.storage[ps->bucket_prev].bucket_next = ps->bucket_next;
67 else
68- *(pshash_getbucket (ps->pid)) = ps->bucket_next;
69+ {
70+ bucket = pshash_getbucket (ps->pid);
71+ *bucket = ps->bucket_next; /* deleting chain head in hash table */
72+ }
73+
74+ /* clear out this cell, just in case */
75+ ps->pid = NO_PID;
76+ ps->bucket_next = ps->bucket_prev = NO_PIDSTAT;
77 }
78
79 static int
80 bgp_delete (pid)
81 pid_t pid;
82 {
83- ps_index_t psi;
84+ ps_index_t psi, orig_psi;
85
86 if (bgpids.storage == 0 || bgpids.nalloc == 0 || bgpids.npid == 0)
87 return 0;
88
89 /* Search chain using hash to find bucket in pidstat_table */
90- for (psi = *(pshash_getbucket (pid)); psi != NO_PIDSTAT; psi = bgpids.storage[psi].bucket_next)
91- if (bgpids.storage[psi].pid == pid)
92- break;
93+ for (orig_psi = psi = *(pshash_getbucket (pid)); psi != NO_PIDSTAT; psi = bgpids.storage[psi].bucket_next)
94+ {
95+ if (bgpids.storage[psi].pid == pid)
96+ break;
97+ if (orig_psi == bgpids.storage[psi].bucket_next) /* catch reported bug */
98+ {
99+ internal_warning ("bgp_delete: LOOP: psi (%d) == storage[psi].bucket_next", psi);
100+ return 0;
101+ }
102+ }
103
104 if (psi == NO_PIDSTAT)
105 return 0; /* not found */
106@@ -904,15 +933,22 @@ static int
107 bgp_search (pid)
108 pid_t pid;
109 {
110- ps_index_t psi;
111+ ps_index_t psi, orig_psi;
112
113 if (bgpids.storage == 0 || bgpids.nalloc == 0 || bgpids.npid == 0)
114 return -1;
115
116 /* Search chain using hash to find bucket in pidstat_table */
117- for (psi = *(pshash_getbucket (pid)); psi != NO_PIDSTAT; psi = bgpids.storage[psi].bucket_next)
118- if (bgpids.storage[psi].pid == pid)
119- return (bgpids.storage[psi].status);
120+ for (orig_psi = psi = *(pshash_getbucket (pid)); psi != NO_PIDSTAT; psi = bgpids.storage[psi].bucket_next)
121+ {
122+ if (bgpids.storage[psi].pid == pid)
123+ return (bgpids.storage[psi].status);
124+ if (orig_psi == bgpids.storage[psi].bucket_next) /* catch reported bug */
125+ {
126+ internal_warning ("bgp_search: LOOP: psi (%d) == storage[psi].bucket_next", psi);
127+ return -1;
128+ }
129+ }
130
131 return -1;
132 }
133diff --git a/patchlevel.h b/patchlevel.h
134index a711c49..4a65dc0 100644
135--- a/patchlevel.h
136+++ b/patchlevel.h
137@@ -25,6 +25,6 @@
138 regexp `^#define[ ]*PATCHLEVEL', since that's what support/mkversion.sh
139 looks for to find the patch level (for the sccs version string). */
140
141-#define PATCHLEVEL 19
142+#define PATCHLEVEL 20
143
144 #endif /* _PATCHLEVEL_H_ */
diff --git a/debian/patches/series b/debian/patches/series
index ee570e7..dab8a2a 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -22,3 +22,4 @@ bzero.diff
22man-macro-warnings.diff22man-macro-warnings.diff
23po-de-fix.diff23po-de-fix.diff
24man-vx-opts.diff24man-vx-opts.diff
25bash44-020.diff

Subscribers

People subscribed via source and target branches