bug#39610: R6RS `flush-output-port` not playing along with `transcoded-port`

bug#39610: R6RS `flush-output-port` not playing along with `transcoded-port`

[Andreas Rottmann]

Hi fellow Schemers!

I was gently nudged on IRC into having a look into my Scheme code, after
many years of abandon, and found that `dorodango` (a reasonably large
body of R6RS code) hangs with Guile 2.2 and 3.0, while it worked on 2.0
(IIRC). I isolated the cause; the following snippet hangs on Guile 2.2
and 3.0, while it worked as expected on 2.0:

;; ------------------
(import (rnrs))

(let* ((p (pipe))
       (in (car p))
       (out (transcoded-port (cdr p) (make-transcoder (utf-8-codec)))))
  (put-datum out "foo")
  (flush-output-port out)
  (display "Should have written to pipe by now, attempting reading a byte\n")
  (display "Got")
  (display (get-u8 in))
  (newline))
;; -------------------

It seems the underlying port is no longer flushed to the OS, so the
`get-u8` now hangs waiting for input, starting with Guile 2.2.

Regards, Rotty

bug#39610: R6RS `flush-output-port` not playing along with `transcoded-port`

[Andreas Rottmann]

Andreas Rottmann <mail@r0tty.org> writes:

> [...] I isolated the cause; the following snippet hangs on Guile 2.2
> and 3.0, while it worked as expected on 2.0:
>
> ;; ------------------
> (import (rnrs))
>
> (let* ((p (pipe))
>        (in (car p))
>        (out (transcoded-port (cdr p) (make-transcoder (utf-8-codec)))))
>   (put-datum out "foo")
>   (flush-output-port out)
>   (display "Should have written to pipe by now, attempting reading a byte\n")
>   (display "Got")
>   (display (get-u8 in))
>   (newline))
> ;; -------------------
>
> It seems the underlying port is no longer flushed to the OS, so the
> `get-u8` now hangs waiting for input, starting with Guile 2.2.
>
I'd like to add that this is indeed not passing data to the OS, as
verified by strace. Also, I have now figured out the commit introducing
the regression, namely 8399e7af5 ("Generic port facility provides
buffering uniformly"); the commit before (e8eeeeb1d) still runs the
above code to completion.

I'd be somewhat motivated to try coming up with a fix, and turn the
above snippet into a testcase, but I guess I could use some hinting in
the right direction.

Regards, Rotty

bug#39610: R6RS `flush-output-port` not playing along with `transcoded-port`

[Ludovic Courtès]

Hi Andreas,

And welcome back!  :-)

Andreas Rottmann <mail@r0tty.org> skribis:

> Andreas Rottmann <mail@r0tty.org> writes:
>
>> [...] I isolated the cause; the following snippet hangs on Guile 2.2
>> and 3.0, while it worked as expected on 2.0:
>>
>> ;; ------------------
>> (import (rnrs))
>>
>> (let* ((p (pipe))
>>        (in (car p))
>>        (out (transcoded-port (cdr p) (make-transcoder (utf-8-codec)))))
>>   (put-datum out "foo")
>>   (flush-output-port out)
>>   (display "Should have written to pipe by now, attempting reading a byte\n")
>>   (display "Got")
>>   (display (get-u8 in))
>>   (newline))
>> ;; -------------------
>>
>> It seems the underlying port is no longer flushed to the OS, so the
>> `get-u8` now hangs waiting for input, starting with Guile 2.2.
>>
> I'd like to add that this is indeed not passing data to the OS, as
> verified by strace. Also, I have now figured out the commit introducing
> the regression, namely 8399e7af5 ("Generic port facility provides
> buffering uniformly"); the commit before (e8eeeeb1d) still runs the
> above code to completion.

Actually I think the code above behaves as expected.  ‘pipe’ returns
buffered ports by default.  When flushing the transcoded port,
‘transcoded_port_write’ is called, but then bytes written to the pipe
are buffered.

The fix is to add:

  (setvbuf (cdr p) 'none)

Does that make sense?

Thanks,
Ludo’.

bug#39610: R6RS `flush-output-port` not playing along with `transcoded-port`

[Andreas Rottmann]

Ludovic Courtès writes:

> Hi Andreas,
>
> And welcome back!  :-)
>
Thanks -- and thanks in return for looking into this!

> Andreas Rottmann <mail@r0tty.org> skribis:
>
>> Andreas Rottmann <mail@r0tty.org> writes:
>>
>>> [...] I isolated the cause; the following snippet hangs on Guile 2.2
>>> and 3.0, while it worked as expected on 2.0:
>>>
>>> ;; ------------------
>>> (import (rnrs))
>>>
>>> (let* ((p (pipe))
>>>        (in (car p))
>>>        (out (transcoded-port (cdr p) (make-transcoder (utf-8-codec)))))
>>>   (put-datum out "foo")
>>>   (flush-output-port out)
>>>   (display "Should have written to pipe by now, attempting reading a byte\n")
>>>   (display "Got")
>>>   (display (get-u8 in))
>>>   (newline))
>>> ;; -------------------
>>>
>>> It seems the underlying port is no longer flushed to the OS, so the
>>> `get-u8` now hangs waiting for input, starting with Guile 2.2.
>>>
>> I'd like to add that this is indeed not passing data to the OS, as
>> verified by strace. Also, I have now figured out the commit introducing
>> the regression, namely 8399e7af5 ("Generic port facility provides
>> buffering uniformly"); the commit before (e8eeeeb1d) still runs the
>> above code to completion.
>
> Actually I think the code above behaves as expected.  ‘pipe’ returns
> buffered ports by default.  When flushing the transcoded port,
> ‘transcoded_port_write’ is called, but then bytes written to the pipe
> are buffered.
>
> The fix is to add:
>
>   (setvbuf (cdr p) 'none)
>
> Does that make sense?
>
It makes sense, and I can confirm that it makes the boiled-down example
I posted work.

However, I'm not sure it is the expected behavior. Regardless of
buffering modes used, I would expect a `flush-output-port` in a "port
stack" (as produced by `transcoded-output-port`) to propagate all the
way to the OS. It seems that was the case in Guile 2.0, as I'm pretty
sure I observed the "breaking" behavior change with 8399e7af5 applied,
and not with the commit preceding it.

If the current behavior is indeed the intended one, we should make sure
the docs somehow reflect this caveat, as I imagine it may surprise
future Guile users which happen to use its R6RS support and pipes in
combination.

Regards, Rotty

bug#39610: R6RS `flush-output-port` not playing along with `transcoded-port`

[Ludovic Courtès]

Hi,

Andreas Rottmann <mail@r0tty.org> skribis:

>> Andreas Rottmann <mail@r0tty.org> skribis:
>>
>>> Andreas Rottmann <mail@r0tty.org> writes:
>>>
>>>> [...] I isolated the cause; the following snippet hangs on Guile 2.2
>>>> and 3.0, while it worked as expected on 2.0:
>>>>
>>>> ;; ------------------
>>>> (import (rnrs))
>>>>
>>>> (let* ((p (pipe))
>>>>        (in (car p))
>>>>        (out (transcoded-port (cdr p) (make-transcoder (utf-8-codec)))))
>>>>   (put-datum out "foo")
>>>>   (flush-output-port out)
>>>>   (display "Should have written to pipe by now, attempting reading a byte\n")
>>>>   (display "Got")
>>>>   (display (get-u8 in))
>>>>   (newline))
>>>> ;; -------------------
>>>>
>>>> It seems the underlying port is no longer flushed to the OS, so the
>>>> `get-u8` now hangs waiting for input, starting with Guile 2.2.
>>>>
>>> I'd like to add that this is indeed not passing data to the OS, as
>>> verified by strace. Also, I have now figured out the commit introducing
>>> the regression, namely 8399e7af5 ("Generic port facility provides
>>> buffering uniformly"); the commit before (e8eeeeb1d) still runs the
>>> above code to completion.
>>
>> Actually I think the code above behaves as expected.  ‘pipe’ returns
>> buffered ports by default.  When flushing the transcoded port,
>> ‘transcoded_port_write’ is called, but then bytes written to the pipe
>> are buffered.
>>
>> The fix is to add:
>>
>>   (setvbuf (cdr p) 'none)
>>
>> Does that make sense?
>>
> It makes sense, and I can confirm that it makes the boiled-down example
> I posted work.
>
> However, I'm not sure it is the expected behavior. Regardless of
> buffering modes used, I would expect a `flush-output-port` in a "port
> stack" (as produced by `transcoded-output-port`) to propagate all the
> way to the OS. It seems that was the case in Guile 2.0, as I'm pretty
> sure I observed the "breaking" behavior change with 8399e7af5 applied,
> and not with the commit preceding it.

Port types don’t have a “flush” operation, only “write”.  Thus, it’s
impossible to define a port type that would propagate flushes.
There are pros and cons I guess, but it seems like a reasonable choice
to me.

When implementing a “proxying” port type like ‘transcoded-port’ that
does its own buffering, it’s probably OK to say that it’s the proxy’s
responsibility to ensure there’s no double-buffering taking place.

> If the current behavior is indeed the intended one, we should make sure
> the docs somehow reflect this caveat, as I imagine it may surprise
> future Guile users which happen to use its R6RS support and pipes in
> combination.

Maybe we should not document this specific combination but rather the
more general issue?  I’m not sure how to do that.  Thoughts?

Thanks,
Ludo’.

bug#39610: R6RS `flush-output-port` not playing along with `transcoded-port`

[Andreas Rottmann]

Ludovic Courtès writes:

> Hi,
>
> Andreas Rottmann <mail@r0tty.org> skribis:
>
>>> Andreas Rottmann <mail@r0tty.org> skribis:
>>>
>>>> Andreas Rottmann <mail@r0tty.org> writes:
>>>>
>>>>> [...] I isolated the cause; the following snippet hangs on Guile 2.2
>>>>> and 3.0, while it worked as expected on 2.0:
>>>>>
>>>>> [...]
>>>>>
>>>>> It seems the underlying port is no longer flushed to the OS, so the
>>>>> `get-u8` now hangs waiting for input, starting with Guile 2.2.
>>>>>
>>>> [...]
>>>
>>> Actually I think the code above behaves as expected.  ‘pipe’ returns
>>> buffered ports by default.  When flushing the transcoded port,
>>> ‘transcoded_port_write’ is called, but then bytes written to the pipe
>>> are buffered.
>>>
>>> The fix is to add:
>>>
>>>   (setvbuf (cdr p) 'none)
>>>
>>> Does that make sense?
>>>
>> It makes sense, and I can confirm that it makes the boiled-down example
>> I posted work.
>>
>> However, I'm not sure it is the expected behavior. Regardless of
>> buffering modes used, I would expect a `flush-output-port` in a "port
>> stack" (as produced by `transcoded-output-port`) to propagate all the
>> way to the OS. It seems that was the case in Guile 2.0, as I'm pretty
>> sure I observed the "breaking" behavior change with 8399e7af5 applied,
>> and not with the commit preceding it.
>
> Port types don’t have a “flush” operation, only “write”.  Thus, it’s
> impossible to define a port type that would propagate flushes.
> There are pros and cons I guess, but it seems like a reasonable choice
> to me.
>
> When implementing a “proxying” port type like ‘transcoded-port’ that
> does its own buffering, it’s probably OK to say that it’s the proxy’s
> responsibility to ensure there’s no double-buffering taking place.
>
In my understanding, the "proxy type" is the R6RS transcoded port in
this case, which has no control over the underlying port, but provides a
flush operation (`flush-output-port`), which R6RS specifies as:

  Flushes any buffered output from the buffer of output-port to the
  underlying file, device, or object. The flush-output-port procedure
  returns unspecified values.

So if I obtain a transcoded port from a pipe port, and call
`flush-output-port` on the transcoded port, I'd expect the bytes to end
up at _least_ at the pipe port. That probably happens currently;
however, the thing is that R6RS also says about `transcoded-port`:

   As a side effect, however, transcoded-port closes binary-port in a
   special way that allows the new textual port to continue to use the
   byte source or sink represented by binary-port, even though
   binary-port itself is closed and cannot be used by the input and
   output operations described in this chapter.

So, I conclude: when you use `transcoded-port` with any underlying Guile
port, and you care about buffering behavior, you _need_ to set the
underlying port's buffer mode to 'none`, _before_ constructing the
transcoded port. I have not tried if Guile enforces the "specially
closed mode", but I am thinking in the context of an abstraction over
`pipe` and `primitive-fork`, intended to provide a basis for "pure" R6RS
code [1], so the client code cannot just call Guile's `setvbuf`, without
losing portability.

[1] https://github.com/rotty/spells/blob/master/spells/process/compat.guile.sls

Do you agree with that conclusion?

  [ After writing the above, I realize that might be what you meant, after
    all: do you propose that, as a fix, `transcoded-port` sets the buffer
    mode of the underlying port to `none`? ]

I must admit that I am confused -- how can a port type have no flush
operation (which is evidently true from looking at scm_t_port_type), and
Guile's `force-output` procedure, generic over port types, still exist?

>> If the current behavior is indeed the intended one, we should make sure
>> the docs somehow reflect this caveat, as I imagine it may surprise
>> future Guile users which happen to use its R6RS support and pipes in
>> combination.
>
> Maybe we should not document this specific combination but rather the
> more general issue?  I’m not sure how to do that.  Thoughts?
>
I now realized I need to wrap my head around the changed port
implementation, and how it relates to R6RS in general, and
`transcoded-port` specifically, before starting to think about
documentation.

I think I have identified another related issue, this time for reading
from `trancoded-port`, but I'd rather make sure we have a shared
understanding of the expected flushing and buffering behavior, before
bringing that up.

Regards, Rotty

bug#39610: R6RS `flush-output-port` not playing along with `transcoded-port`

[Andreas Rottmann]

[ re-send, used gmane to post a follow-up on the initial bug report, and
  realized that this likely cannot work. Apologies if I'm wrong and this
  ends up as a duplicate. ]

Andreas Rottmann <mail@r0tty.org> writes:

> [...] I isolated the cause; the following snippet hangs on Guile 2.2
> and 3.0, while it worked as expected on 2.0:
>
> ;; ------------------
> (import (rnrs))
>
> (let* ((p (pipe))
>        (in (car p))
>        (out (transcoded-port (cdr p) (make-transcoder (utf-8-codec)))))
>   (put-datum out "foo")
>   (flush-output-port out)
>   (display "Should have written to pipe by now, attempting reading a byte\n")
>   (display "Got")
>   (display (get-u8 in))
>   (newline))
> ;; -------------------
>
> It seems the underlying port is no longer flushed to the OS, so the
> `get-u8` now hangs waiting for input, starting with Guile 2.2.
>
I'd like to add that this is indeed not passing data to the OS, as
verified by strace. Also, I have now figured out the commit introducing
the regression, namely 8399e7af5 ("Generic port facility provides
buffering uniformly"); the commit before (e8eeeeb1d) still runs the
above code to completion.

I'd be somewhat motivated to try coming up with a fix, and turn the
above snippet into a testcase, but I guess I could use some hinting in
the right direction.

Regards, Rotty