Re: [AC 21.5] Prevent SIGPIPE in deactivate_process
Stephen J. Turnbull wrote:
Mike Kupfer writes:
> I agree that XEmacs needs to find out about the condition and clean up.
> But my understanding is that the SIGPIPE is only generated when XEmacs
> tries to write to the pipe. AFAIK, XEmacs doesn't need the asynchronous
> notification that a signal provides; it can just check for an error
> return from write() (EPIPE).
Well, in my (imperfect) understanding, in theory that's true, but it
would require a new Lstream implementation for pipes, or maybe changes
to the current filedesc implementation (which doesn't expect EPIPE).
Also, some amount of higher-level code would need to be rewritten and
reviewed for race conditions (it is designed to assume a reliable
stream, and to ignore the details of when data is actually written to
the pipe except when an explicit flush is called for).
But the Lstream abstraction already requires the caller to check for an
error code, for both the write and the flush. unix_send_process() does
this SIGPIPE dance, with a flush at the end; it could easily check for
an error return instead. AFAICS, the only benefit to the current
structure is that it lets us mark the output stream stream as closed and
mark the subprocess as dead, all in one place in the code. And I'm not
sure that being able to mark the output stream as closed there is really
an advantage, as it smells like a layering (abstraction) violation to
me.
Except there's a third approach (the one actually implemented in
XEmacs), which is to provide non-fatal SIGPIPE handling. The problem
is that it's buggy.
True, but the reason it's buggy is that it tries to predict when the
Lstream code might push bits out to the pipe, and it only has the signal
catcher enabled in those places. That, too, smells like a layering
violation.
Though I suppose this points at yet another possible fix: change
unix_send_process() to register its SIGPIPE handler after doing the
setjmp, and only restore the old handler just before returning.
I still think a cleaner solution is possible without a major rewrite to
the Lstream code, or a separate Lstream implementation for pipes. Just
have the Lstream code ignore SIGPIPE, and have it pass EPIPE failures up
to the caller. unix_send_process() would need some changes, but IMO
that would mostly be getting rid of unnecessary complexity.
If that's a larger change than you want to tackle at this time, I can't
argue with that. It's your time, after all. :-) Changing
unix_send_process() so that it sets the SIGPIPE handler just once would
be my second choice. And I don't really object to your
close-without-flush solution, either, at least as a short-term fix.
cheers,
mike
_______________________________________________
XEmacs-Patches mailing list
XEmacs-Patches(a)xemacs.org
http://lists.xemacs.org/mailman/listinfo/xemacs-patches