hhvm / hsl-experimental

Experimental features for the Hack Standard Library
MIT License
23 stars 10 forks source link

[Network] allow setting socket options #106

Open azjezz opened 4 years ago

azjezz commented 4 years ago

see: https://github.com/facebook/hhvm/issues/8451

fredemmott commented 4 years ago

Unless I'm missing something, the HHVM issue is not relevant here - the sockets are created with socket_create, which means that TCP_NODELAY and SO_KEEPALIVE can be set with socket_set_option

azjezz commented 4 years ago

HHVM doesn't support no-dely / keep-alive :disappointed:

fredemmott commented 4 years ago

That doesn't appear to be the case

hphpd> $sock = socket_create(AF_INET, SOCK_STREAM, SOL_TCP)
hphpd> socket_set_option($sock, SOL_SOCKET, /* random number */ 12345, 1)
Warning: unable to set socket option [42]: Protocol not available
hphpd> =socket_get_option($sock, SOL_SOCKET, SO_KEEPALIVE)
0
hphpd> =socket_get_option($sock, SOL_TCP, TCP_NODELAY)
=socket_get_option($sock, SOL_TCP, TCP_NODELAY)
0
hphpd> socket_set_option($sock, SOL_SOCKET, SO_KEEPALIVE, 1)
hphpd> socket_set_option($sock, SOL_TCP, TCP_NODELAY, 1)
hphpd> =socket_get_option($sock, SOL_SOCKET, SO_KEEPALIVE)
8
hphpd> =socket_get_option($sock, SOL_TCP, TCP_NODELAY)
4
fredemmott commented 4 years ago

that said, retitled as I won't be doing these in small batches; there's many SO and TCP options, and I'm hoping to avoid adding something as rough as mixed, but that might end up being necessary. At the least, there should be an enum for socket options, a different enum for tcp options, etc.

azjezz commented 4 years ago

That doesn't appear to be the case

:scream: i clearly remember they didn't exist back when i was working on #36

At the least, there should be an enum for socket options, a different enum for tcp options, etc.

wouldn't using a shape be "stricter" ? e.g, keep-alive value should be bool, while timeout should be float

fredemmott commented 4 years ago

perhaps - probably separate shapes, then TCP\Conenct would take shape('socket_options' => SocketOptionsShape', 'tcp_options' => TcpOptionsShape) or something like that or options -> socket, options -> tcp

azjezz commented 4 years ago

or maybe tcp options shape can have all socket options therefore it can be passed as both ? :)

fredemmott commented 4 years ago

while timeout should be float

When we move to native implementaitons, I'm probably going to change all timeouts to be ints, or something like the timeval C struct - the native implementaiton is an int seconds and int microseconds. It's not as convenient, but avoids issues like https://github.com/hhvm/hsl-experimental/pull/104

or maybe tcp options shape can have all socket options therefore it can be passed as both ? :)

fredemmott commented 4 years ago

😱 i clearly remember they didn't exist back when i was working on #36

Looks like they were added in 2015; for both #36 and the HHVM issue, you were specific about the stream_socket_*() functions - they still appear to be unsupported for stream_context_set_option() - could be this is what you were referring to, instead of socket_set_option() ?

azjezz commented 4 years ago

probably :thinking:

fredemmott commented 4 years ago

Splat would be really nice here; there's a few (SO_NREAD, SO_ERROR, SO_NWRITE in particular) that it would be nice to type as invalid for setsockopt, but valid for getsockopt, without duplciating the entire shape