Skip to content

Fix duplicated emission and implement graceful shutdown - #1

Closed
mpyw wants to merge 4 commits into
boite:developfrom
mpyw-forks:tls-support
Closed

Fix duplicated emission and implement graceful shutdown#1
mpyw wants to merge 4 commits into
boite:developfrom
mpyw-forks:tls-support

Conversation

@mpyw

@mpywmpyw commented Aug 31, 2016

Copy link
Copy Markdown

I am using your great commit in mpyw/php-hyper-builtin-server, thank you!!

However, there are some bugs to be fixed.

Comment threadsrc/Server.php
$scope = $this;
$connection->on('connection', function ($dataConn) use ($scope) {
$scope->emit('connection', array($dataConn));
});

@mpywmpywAug 31, 2016

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

It seems to be duplicated to

$this->emit('connection', array($this));

if (! $this->isSecure) {
$enabled = stream_socket_enable_crypto($stream, true, $this->protocolNumber);
if ($enabled === false) {
$this
->err('Failed to complete a secure handshake with the client.')
->end()
;
return;
} elseif ($enabled === 0) {
return;
}
$this->isSecure = true;
$this->emit('connection', array($this));
}

Actually I verified responses are duplicated in my product mpyw/php-hyper-builtin-server and my commit fixes the issue.

@mpywmpyw changed the title TLS supportFix duplicated emission and implement graceful shutdownAug 31, 2016
if (false === stream_socket_enable_crypto($stream, false)) {
$scope->err('Failed to gracefully shutdown a secure connection.');
}
});

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

This is required for graceful shutdown

@boite

Copy link
Copy Markdown
Owner

Thank you @mpyw. I will review these changes as soon as I can.

@mpywmpyw closed this Apr 12, 2017
@mpyw
mpyw deleted the tls-support branch April 12, 2017 11:17
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@mpyw@boite