[win] decode_frames

[ZeeWanderer]

No description provided.

[facebook-github-bot]

@robertoaloi has imported this pull request. If you are a Meta employee, you can view this in D81490753.

[jcpetruzza]

@ZeeWanderer I'm still trying to understand why this is needed on windows. E.g., is this a bug in binary:split/2 or something is messing around with stdin/stdout?

If you try this on a windows box, what do you see:

$ erl
Erlang/OTP 27 [erts-15.1] [source-74032c2831] [64-bit] [smp:36:36] [ds:36:36:10] [async-threads:1] [jit:ns] [systemtap]

Eshell V15.1 (press Ctrl+G to abort, type help(). for help)
1> binary:split(~"foo\r\n\r\n42", ~"\r\n\r\n").
[<<"foo">>,<<"42">>]
2> 

[ZeeWanderer]

i see [<<"foo">>,<<"42">>]. As far as i understand it's a bug with stdin/stdout. Basically pipe mode is inherited, and there is a bug where despite the erl opening the pipe as binary it inherits the text mode because windows. And in text mode \r\n -> \n happens automatically.

[jcpetruzza]

As far as i understand it's a bug with stdin/stdout. Basically pipe mode is inherited

Ah, this is a good insight! In edb_dap_transport we open the port on stdin/stdout but by default they are, as you say, in text mode. So changing stdin/stdout to binary mode before opening the port should do the trick. Could you verify if #11 fixes this issue on windows?

[ZeeWanderer]

@jcpetruzza i have tried adding that and it does not solve the issue. The bug appears to be in the erl<->windows interop specifically. Any console apps by default have text mode for the pipes and erl seems to be unable to switch it to binary by any means i have tried

[jcpetruzza]

@ZeeWanderer Interesting, is there a bug filed for this already? I didn't find anything on the otp issue tracker. If not, could we make a small repro and file one? We can then add the link to the issue as a comment on top of your mitigation

[ZeeWanderer]

@jcpetruzza i havent found a bug filed either

[ZeeWanderer]

What is the status on this?

[jcpetruzza]

The theory is that there is a bug in the windows version of OTP when a port is created on the stdin/stdout descriptors. I'd like to have a small repro of the problem on windows so that we can open an issue on OTP. With the issue open, I'm more comfortable merging this as a temporary workaround.

I'm a bit blocked getting hold of a windows box where I can try this out myself, though. @ZeeWanderer if you can try to make the repro that would help: essentially, open a port like we do in edb_dap_transport:init(), check that when you send "\r\n" you receive instead "\n\n" even though everything is set to binary.

[ZeeWanderer]

%% File: stdio_port_repro.erl
-module(stdio_port_repro).
-export([run/0, child/0]).

run() ->
    {ok, Erl} = find_erl(),
    Port = open_port(
      {spawn_executable, Erl},
      [{args, ["-noshell","-noinput","-s","stdio_port_repro","child"]},
       use_stdio, stderr_to_stdout, exit_status, binary]
    ),
    wait_ready(Port),
    lists:foreach(
      fun({Name, Payload}) -> do_test(Port, Name, Payload) end,
      [{"CRLF", <<13,10>>}, {"LF", <<10>>}, {"CR", <<13>>}]
    ),
    port_close(Port),
    receive {Port, {exit_status, _}} -> ok after 1000 -> ok end.

find_erl() ->
    case os:find_executable("erl") of
        false -> exit(no_erl_in_path);
        Path  -> {ok, Path}
    end.

wait_ready(Port) ->
    receive
        {Port, {data, <<"READY">>}} -> ok;
        {Port, _} -> wait_ready(Port)
    after 5000 ->
        exit(timeout_wait_ready)
    end.

do_test(Port, Name, Payload) ->
    port_command(Port, Payload),
    Echo = receive {Port, {data, Bin}} -> Bin after 2000 -> timeout end,
    io:format("~s sent: ~s echoed: ~s~n", [Name, hex(Payload), hex(Echo)]),
    case Echo of
        Payload -> io:format("~s => OK~n", [Name]);
        _       -> io:format("~s => MISMATCH~n", [Name])
    end.

hex(timeout) -> <<"timeout">>;
hex(Bin) when is_binary(Bin) ->
    iolist_to_binary([io_lib:format("~2.16.0B ", [B]) || <<B:8>> <= Bin]).

child() ->
    P = open_port({fd, 0, 1}, [binary, stream, eof]),
    port_command(P, <<"READY">>),
    loop(P).

loop(P) ->
    receive
        {P, {data, Bin}} -> port_command(P, Bin), loop(P);
        {P, eof} -> ok
    end.

CRLF sent: 0D 0A  echoed: 0A
CRLF => MISMATCH
LF sent: 0A  echoed: 0A
LF => OK
CR sent: 0D  echoed: 0A
CR => MISMATCH

[jcpetruzza]

Interesting. Of course this works as expected on linux:

CRLF sent: 0D 0A  echoed: 0D 0A 
CRLF => OK
LF sent: 0A  echoed: 0A 
LF => OK
CR sent: 0D  echoed: 0D 
CR => OK

In child() could you add a io:setopts([binary]) before opening the port?

[ZeeWanderer]

added and recompiled
result stays the same

CRLF sent: 0D 0A  echoed: 0A
CRLF => MISMATCH
LF sent: 0A  echoed: 0A
LF => OK
CR sent: 0D  echoed: 0A
CR => MISMATCH

[jcpetruzza]

Perfect, could I ask you to file the bug in otp, as you can provide better instructions to reproduce? (windows version, otp build info, etc)

[jcpetruzza]

@ZeeWanderer we've been discussing this issue, and changing \r\n\r\n for \n\n on windows is unfortunately not enough. The problem is that the DAP specification also requires that each frame comes with a "Content-Length`, and due to the conversion being done, whenever there are newlines involved, the declared length and the actual lengths will not match.

So edb windows support is effectively blocked until erlang/otp#10258 is resolved