[#518] Replace group leader when using stdio transport

[jfacorro]

Description

Replace group leader when using stdio transport.

Fixes #518.

[alanz]

LGTM

I saw in reading that the group leader is abused to harvest processes when shutting down. Will that be compromised by this? Will it matter, given we shut down the O/S process as a whole when terminating the server?

[jfacorro]

I saw in reading that the group leader is abused to harvest processes when shutting down. Will that be compromised by this?

I don't think it will. The processes that wouldn't be killed are the ones that are not under the supervision tree, as long as every process we spawn is linked to a process in the supervision tree it should be fine.

Will it matter, given we shut down the O/S process as a whole when terminating the server?

I agree that it ultimately does not matter for the users. If by any chance some dangling process is still alive when the application is stopped, this could affect some of the tests, depending on what the process does. But this would still happen without the change in this PR. @robertoaloi was having some issues with the process that keeps the TCP connection in his indexing PR.

[alanz]

Running this under emacs, I get

LSP :: Connected to [erlang-ls:14246 status:starting].
LSP :: erlang-ls:14246 initialized successfully
Error processing message (lsp-unknown-message-type #s(hash-table size 65 test equal rehash-size 1.5 rehash-threshold 0.8125 data ("error" #s(hash-table size 65 test equal rehash-size 1.5 rehash-threshold 0.8125 data ("code" -32001 "message" "Unexpected error while initialized")) "id" nil "jsonrpc" "2.0"))).

Bisecting blames 9050e6a

[robertoaloi]

If possible, I would appreciate @dszoboszlay 's input on this one.

[alanz]

BTW, the error in emacs is complaining about this line getting hit

erlang_ls/src/els_methods.erl

Line 73 in 9050e6a

, message => <<"Unexpected error while ", Method/binary>>

[alanz]

My setup is mac, catalina, emacs GNU Emacs 26.3 (build 1, x86_64-apple-darwin18.2.0, NS appkit-1671.20 Version 10.14.3 (Build 18D109)) of 2019-09-02

Erlang/OTP 22 [erts-10.6.4] [source] [64-bit] [smp:16:16] [ds:16:16:10] [async-threads:1] [hipe] [dtrace]

[jfacorro]

I found the problem! We are using application:get_application() which is relying on the group_leader():

-spec get_application() -> 'undefined' | {'ok', Application} when
      Application :: atom().
get_application() -> 
    application_controller:get_application(group_leader()).

[dszoboszlay]

Isn't this the opposite of what you want? You call restrict_stdio_access() for every transport except for els_stdio.

[robertoaloi]

It looks like @dszoboszlay is correct. I also suggest we do not rely on the andalso semantics for this, since it makes the code less readable. I would rather:

restrict_stdio_access(els_stdio) ->
  restrict_stdio_access();
restrict_stdio_access(_) ->
  ok.

[jfacorro]

🤦

[jfacorro]

Fixed by #532

[dszoboszlay]

All in all, I don't think you should base your solution on changing the group_leader in one supervisor.

• It prevents application:get_application() from working properly.
• If you start a different application with a supervisor tree, this solution will not block that application's processes from writing to the standard output.

I'd recommend taking a completely different approach:

• Start Erlang with the -nouser command line argument. This will tell the kernel not to start a user process, basically blocking access to the standard_io I/O device.
• You can still access the standard output of the OS process by opening {fd, 0, 1} as a port:

P = erlang:open_port({fd, 0, 1}, []),
erlang:port_command(P, <<"hello world!\n">>),
erlang:port_close(P).

It is very unlikely that any other library would accidentally write to this port! 😄