[OpenWrt-Devel] [PATCH procd] service: use PTY (instead of a pipe) for reading service output

[Yousong Zhou]

On Jun 19, 2015 9:16 PM, "Rafał Miłecki" <zajec5 at gmail.com> wrote:
>
> On 19 June 2015 at 14:44, Yousong Zhou <yszhou4tech at gmail.com> wrote:
> > On 19 June 2015 at 19:34, Rafał Miłecki <zajec5 at gmail.com> wrote:
> >> Using pipe automatically switches service to block buffering which kind
> >> of breaks our logging. We won't get anything from FD until the buffer
> >> gets filled fully or the service exits. This makes log messages appear
> >> with an unwanted delay.
> >> Switching to PTY fixes this issue. Service prints to the "standard"
> >> stdout (no piping) but the messages go (line buffered) to the master FD
> >> anyway.
> >> It was successfully tested with all 4 variants of "stdout" and "stderr"
> >> ubus arguments.
> >>
> >
> > I do not like this because
> >
> >  - Conventionally, services will drop their inherited controlling
> > terminal for preparation of becoming a daemon.  Adding back that
> > controlling terminal by login_tty() for alleviating possible delay
> > when logging stdio sounds not right.
>
> Do you have any other idea?
> The delay isn't "possible", I think it occurs with 90%+ of apps. I'm
> not aware of many apps (except for grep maybe) that calls "fflush"
> after every printf. It simply makes reading real-time logs with
> "logread" not working at all.
>
>
> >  - I am biased... in that the ossherd [1] package depends on the fact
> > processes started by procd do not have controlling terminals and this
> > will unattended password login with ssh  :)
>
> I'm afraid I don't understand this part. In any case, you don't have
> to use logging in your init.d script. It's an option.
>

It's a hacky init script trying to start SOCKS5 proxies with
openssh-client.  Password authentication has to be used in this case.

>
> > Anyway, the implementation is not complete.
> >
> >  - epipe[] is still there.
>
> It is supposed to be. That's the only way to keep logging of stdout
> and stderr separated. Read my comment (in the code) about epipe.
>
>
> >  - aslave needs to be closed when login_pty() failed
>
> I'll fix that.
>
>
> >  - login_pty() and dup2() used together
>
> That's correct, expected, on purpose.
>

but login_pty will do dup2 on all 3 fd's with aslave.  it's duplicate work
and has side effects (regarding a pty stdin).

I think currently not many programs need procd_set_param stdout.  most of
the time if not always stderr, apart from syslog, is used for logging and
error reporting.  If that is agreed, then we can conclude that it is at
least a relatively rare case that we need to log stdout of the program, and
if that is the case we can afford and should patch the program itself to
use line buffered io with stdout.

Regards

                    yousong

>
> >  - aslave should be closed in parent
>
> Like closefd(aslave) which my patch does?

sorry, i missed that one before.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.infradead.org/pipermail/openwrt-devel/attachments/20150619/6b563040/attachment.htm>
-------------- next part --------------
_______________________________________________
openwrt-devel mailing list
openwrt-devel at lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel