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

[Rafał Miłecki]

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.


> 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.


>  - aslave should be closed in parent

Like closefd(aslave) which my patch does?
_______________________________________________
openwrt-devel mailing list
openwrt-devel at lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel