Re: Bug#739989: debian-installer-utils: log-output change breaks speech synthesis
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 <cjwatson@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 [cjwatson@debian.org]
Reply to: