Skip to content
This repository has been archived by the owner on Oct 1, 2023. It is now read-only.

[WIP] Network API Implementation #36

Closed
wants to merge 92 commits into from
Closed

Conversation

azjezz
Copy link
Contributor

@azjezz azjezz commented Feb 15, 2019

todo :

  • implement HH\Lib\Network\NativeSocketHandle
  • implement HH\Lib\Network\TcpServer
  • implement HH\Lib\Network\TcpSocket
  • implement HH\Lib\Network\UdpSocket
  • write tests
  • clean API
  • more things

@facebook-github-bot facebook-github-bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label Feb 15, 2019
@azjezz azjezz changed the title [WIP] Network Implementation [WIP] Network API Implementation Feb 15, 2019
@fredemmott
Copy link
Contributor

Won't be able to look into this in detail for a while, but:

  • no .hack files in this repository for now
  • prefer use namespace
  • it's probably going to work out much better to leave out everything TLS-related for now; right now this needs a review of a network API, an encryption API, and a trust API. Doing it as one means nothing will land until they're all OK.

@azjezz
Copy link
Contributor Author

azjezz commented Feb 15, 2019

thanks for your input @fredemmott 👍

Copy link
Contributor

@fredemmott fredemmott left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also, .php files only, and why are there hhi files?

src/network/socket/Socket.hh Outdated Show resolved Hide resolved
src/network/socket/SocketHandle.hh Outdated Show resolved Hide resolved
src/network/socket/SocketHandle.hh Outdated Show resolved Hide resolved
src/network/socket/Socket.hh Outdated Show resolved Hide resolved
@azjezz
Copy link
Contributor Author

azjezz commented Feb 15, 2019

@fredemmott hhi files are to be implemented.

@azjezz
Copy link
Contributor Author

azjezz commented Feb 20, 2019

currenlty i'm having problems with HH\Lib\_Private\NativeSocketHandle::isEndOfFile()

this would work :

use namespace HH\Lib\Experimental\IO;
use namespace HH\Lib\Experimental\Network;

require __DIR__ . '/../vendor/autoload.hack';

<<__EntryPoint>>
async function tcp_client(): Awaitable<void> {
  Facebook\AutoloadMap\initialize();

  $stdout = IO\server_output();
  $client = await Network\TcpSocket::connect(
    Network\host('www.google.com'),
    80
  );

  try {
    $request = '';
    $request .= 'GET / HTTP/1.1'."\r\n";
    $request .= 'Host: www.google.com'."\r\n\r\n";
    await $client->writeAsync($request);

    $response = await $client->readAsync(8192);

    await $stdout->writeAsync($response . "\n");

  } catch (Exception $e) {
    await $client->closeAsync();
    throw $e;
  }
  await $client->closeAsync();
  await $stdout->closeAsync();
}

but if we change readAsync(8192) to readAsync(), it wouldn't work ( reason : eof always returns false )

@azjezz
Copy link
Contributor Author

azjezz commented Feb 25, 2019

currently, this :

use namespace HH\Lib\Experimental\IO;
use namespace HH\Lib\Experimental\Network;

require __DIR__ . '/../vendor/autoload.hack';

<<__EntryPoint>>
async function tcp_client(): Awaitable<void> {
 Facebook\AutoloadMap\initialize();

 $client = await Network\TcpSocket::connect(
   Network\host('www.google.com'),
   80
 );

 try {
   $request = '';
   $request .= 'GET / HTTP/1.1'."\r\n";
   $request .= 'Connection: close'."\r\n";
   $request .= 'Host: www.google.com'."\r\n\r\n";
   await $client->writeAsync($request);
   await $client->flushAsync();
   $response = await $client->readAsync();
   echo $response;

 } catch (Exception $e) {
   await $client->closeAsync();
   throw $e;
 }
 await $client->closeAsync();
}

would work as expected, but if we change connection to keep-alive it would block until peer drops connection.

keep-alive and no-dely are not supported with sockets created using stream_socket_* functions.

hhvm issue to add these : facebook/hhvm#8451

@fredemmott
Copy link
Contributor

Thanks; this is going to influence the implementation, but not going to be merging as-is: it's likely to be many smaller changes building up to a similar end state

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CLA Signed Do not delete this pull request or issue due to inactivity.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants