clone 739989 -1 -2 reassign -1 espeakup: rebuild against latest espeak severity -1 important reassign -2 espeak: build proper library udeb so that espeakup doesn't have to be statically linked and hence break on upstream version changes severity -2 important thanks
On Mon, Feb 24, 2014 at 09:17:19PM +0300, Cyril Brulebois wrote: > the change below breaks speech synthesis, as reported here: > https://lists.debian.org/debian-accessibility/2014/02/msg00093.html > > and suspected here: > https://lists.debian.org/debian-accessibility/2014/02/msg00102.html > > I've checked that building d-i against testing udebs lets me reproduced > this issue, and that adding d-i-utils' binaries to localudebs after > having reverted this change fixes speech synthesis. > > commit 9c6685364a56697ea9c590e1bc93a73ade88b679 > Author: Colin Watson <cjwat...@debian.org> > Date: Fri Feb 7 17:07:46 2014 +0000 > > log-output: Always install a no-op SIGCHLD handler > > This copes with the case where the subsidiary process starts a daemon > which > does not fully disconnect its standard file descriptors (LP: #1021293). > > See also the changelog for 1.46. > > (http://anonscm.debian.org/gitweb/?p=d-i/debian-installer-utils.git;a=commit;h=9c6685364a56697ea9c590e1bc93a73ade88b679) OK. Thanks for the notice. So, this change was in response to a bug that was pretty painful to reproduce (referenced above, https://launchpad.net/bugs/1021293), so while I understand the need to fix the regression, I certainly wouldn't want to just revert without chasing it down properly. As indicated by the comments, I knew at the time that this was a bit of a shonky fix. The effect is essentially to disconnect from the subsidiary process after it exits, even if it has spawned some child processes. It was extending a workaround that was previously only active with the --pass-stdout option; it was technically wrong there in much the same way, but the corner cases are very rare so we've lived with it quite happily for nearly seven years. Some strace analysis later: what's happening here is that espeakup.restart runs "log-output -t espeakup espeakup -V en", the subsidiary "espeakup -V en" forks, exits, and the child setsids but does *not* do the usual daemonisation thing of redirecting stdin/stdout/stderr to /dev/null (it calls "daemon(0, 1)"). This then tries to print the following to stderr: Wrong version of espeak-data 0x14709 (expects 0x14600) at /usr/lib/i386-linux-gnu/espeak-data Since stderr is now disconnected, this causes espeakup to get SIGPIPE, at which point everything falls over. Oops. (I'm assuming that it would normally have gone on to close those file descriptors or similar a bit later, otherwise this would never have worked at all.) Reuploading espeakup against the latest version of espeak (changelog date 2013-06-19 - how did it take us so long to notice this?) should get rid of this stderr output, clearly ought to happen anyway, and may well work around this bug, so I'm cloning this bug for that. The situation where we have to occasionally rebuild espeakup due to new upstream versions of espeak is clearly far from ideal, and I'm making another clone for that. That still leaves the log-output problem, and in some ways it's amazing it took us so long to run into this. It's obviously wrong to leave a program running with disconnected file descriptors; we only did that because I couldn't think of a better option. However, one trick does come to mind: if we get SIGCHLD to indicate that the child has exited, then log-output could daemonise itself to match (this time a full daemonisation, reopening its standard file descriptors onto /dev/null), and only exit for real once the other ends of the polled pipes close. I think this is actually the correct answer. The problem is that there's no straightforward way to implement this: log-output uses di_exec, there's no hook in that at the point where it's received SIGCHLD, and it would clearly be inappropriate for library code to daemonise. We could maybe figure out some gross hack with the existing handlers, but it would be very fragile. We could add another handler, but it would be hard to do that without breaking ABI or ending up proliferating the di_exec* family of functions even further; the approach of doing this all in a single gigantic function call is not really a very good design but it would be a pain to change now. I am at least half-tempted to just clone and hack the relevant bits of di_internal_exec into log-output, given that its use of it is rather different from most other uses of di_exec* in d-i, and then log-output could do whatever it wants on SIGCHLD. That seems like it should require some discussion rather than JFDI, though. What do people think? In the meantime, I'm reverting my change from 1.103 for now since this is all pretty tricky, although I'm fairly convinced that this is just masking other bugs. So be it, I suppose. Cheers, -- Colin Watson [cjwat...@debian.org] -- To UNSUBSCRIBE, email to debian-bugs-rc-requ...@lists.debian.org with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
