Skip to content
This repository has been archived by the owner on Dec 16, 2019. It is now read-only.

Added method Socket::shutdown() #845

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions classes/socket.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,9 @@ PHP_METHOD(Socket, getLastError);
PHP_METHOD(Socket, clearError);
PHP_METHOD(Socket, strerror);

#ifdef HAVE_SHUTDOWN
Copy link
Contributor

@dktapps dktapps Mar 1, 2018

Choose a reason for hiding this comment

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

where is this macro defined? never mind

PHP_METHOD(Socket, shutdown);
#endif
PHP_METHOD(Socket, close);

ZEND_BEGIN_ARG_INFO_EX(Socket___construct, 0, 0, 3)
Expand Down Expand Up @@ -137,6 +140,12 @@ ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(Socket_strerror, 0, 1, IS_STRING, 1)
ZEND_ARG_TYPE_INFO(0, error, IS_LONG, 0)
ZEND_END_ARG_INFO()

#ifdef HAVE_SHUTDOWN
ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(Socket_shutdown, 0, 1, _IS_BOOL, 0)
ZEND_ARG_TYPE_INFO(0, how, IS_LONG, 0)
ZEND_END_ARG_INFO()
#endif

ZEND_BEGIN_ARG_INFO_EX(Socket_void, 0, 0, 0)
ZEND_END_ARG_INFO()

Expand All @@ -162,6 +171,9 @@ zend_function_entry pthreads_socket_methods[] = {
PHP_ME(Socket, getPeerName, Socket_getHost, ZEND_ACC_PUBLIC)
PHP_ME(Socket, getSockName, Socket_getHost, ZEND_ACC_PUBLIC)
PHP_ME(Socket, close, Socket_void, ZEND_ACC_PUBLIC)
#ifdef HAVE_SHUTDOWN
PHP_ME(Socket, shutdown, Socket_shutdown, ZEND_ACC_PUBLIC)
#endif
PHP_ME(Socket, getLastError, Socket_getLastError, ZEND_ACC_PUBLIC)
PHP_ME(Socket, clearError, Socket_void, ZEND_ACC_PUBLIC)
PHP_ME(Socket, strerror, Socket_strerror, ZEND_ACC_PUBLIC|ZEND_ACC_STATIC)
Expand Down Expand Up @@ -399,6 +411,19 @@ PHP_METHOD(Socket, strerror) {
pthreads_socket_strerror(error, return_value);
} /* }}} */

#ifdef HAVE_SHUTDOWN
/* {{{ proto bool Socket::shutdown(int how) */
PHP_METHOD(Socket, shutdown) {
zend_long how_shutdown = 0;

if (zend_parse_parameters(ZEND_NUM_ARGS(), "l", &how_shutdown) != SUCCESS) {
Copy link
Collaborator

@tpunt tpunt Mar 1, 2018

Choose a reason for hiding this comment

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

Is there any reason in particular why we don't default this parameter to 2? I would assume that most of the time a socket is shut down, we'd like to close it for both reads and writes, so following PHP's socket_shutdown function seems like it would make sense.

RETURN_FALSE;
}

pthreads_socket_shutdown(getThis(), how_shutdown, return_value);
} /* }}} */
#endif

/* {{{ proto bool Socket::close(void) */
PHP_METHOD(Socket, close) {
if (zend_parse_parameters_none() != SUCCESS) {
Expand Down
2 changes: 2 additions & 0 deletions examples/stub.php
Original file line number Diff line number Diff line change
Expand Up @@ -689,6 +689,8 @@ public function getPeerName(bool $port = true) : array{}
public function getSockName(bool $port = true) : array{}

public function close(){}

public function shutdown(int $how) : bool{}

public function getLastError(bool $clear = false){}

Expand Down
17 changes: 17 additions & 0 deletions src/socket.c
Original file line number Diff line number Diff line change
Expand Up @@ -589,6 +589,23 @@ void pthreads_socket_close(zval *object, zval *return_value) {
threaded->store.sock->fd = PTHREADS_INVALID_SOCKET;
}

#ifdef HAVE_SHUTDOWN
void pthreads_socket_shutdown(zval *object, zend_bool how_shutdown, zval *return_value) {
Copy link
Contributor

Choose a reason for hiding this comment

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

the signature here is incorrect - how_shutdown should be zend_long not zend_bool.

pthreads_object_t *threaded =
PTHREADS_FETCH_FROM(Z_OBJ_P(object));

PTHREADS_SOCKET_CHECK(threaded->store.sock);

if (shutdown(threaded->store.sock->fd, how_shutdown) != SUCCESS) {
PTHREADS_SOCKET_ERROR(threaded->store.sock, "Unable to shutdown socket", errno);

RETURN_FALSE;
}

RETURN_TRUE;
}
#endif

void pthreads_socket_set_blocking(zval *object, zend_bool blocking, zval *return_value) {
pthreads_object_t *threaded =
PTHREADS_FETCH_FROM(Z_OBJ_P(object));
Expand Down
3 changes: 3 additions & 0 deletions src/socket.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,9 @@ void pthreads_socket_read(zval *object, zend_long length, zend_long flags, zend_
void pthreads_socket_write(zval *object, zend_string *buf, zend_long length, zval *return_value);
void pthreads_socket_send(zval *object, zend_string *buf, zend_long length, zend_long flags, zval *return_value);
void pthreads_socket_close(zval *object, zval *return_value);
#ifdef HAVE_SHUTDOWN
void pthreads_socket_shutdown(zval *object, zend_bool how_shutdown, zval *return_value);
Copy link
Contributor

Choose a reason for hiding this comment

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

same as above (signature is incorrect)

#endif
void pthreads_socket_set_blocking(zval *object, zend_bool blocking, zval *return_value);
void pthreads_socket_get_peer_name(zval *object, zend_bool port, zval *return_value);
void pthreads_socket_get_sock_name(zval *object, zend_bool port, zval *return_value);
Expand Down
32 changes: 32 additions & 0 deletions tests/socket-shutdown.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
--TEST--
Test of Socket::shutdown() - testing params and separate shutdown of read and write channel
--DESCRIPTION--
Test that creating and closing sockets works as expected on all platforms (gh issue #798)
Copy link
Contributor

Choose a reason for hiding this comment

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

this description might need fixing

--SKIPIF--
<?php
if(!method_exists(\Socket::class, 'shutdown')) {
die('skip.. Socket::shutdown() doesn\'t exist');
}
--FILE--
<?php
$socket = new \Socket(\Socket::AF_INET, \Socket::SOCK_STREAM, 0);
$socket->shutdown();

var_dump($socket->shutdown(4)); // invalid - false
Copy link
Contributor

Choose a reason for hiding this comment

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

wouldn't an exception be more appropriate here?

Copy link
Contributor

Choose a reason for hiding this comment

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

On Windows, this causes an ENOTCONN error to be emitted, which causes a RuntimeException to be thrown. I assume that in Winsock2, socket state is checked prior to checking the argument value [citation needed].

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@dktapps I slightly modified the test from bind()/listen() to connect(). Can you check the test on a windows machine?


$socket->bind('0.0.0.0');
$socket->listen(1);

var_dump($socket->shutdown(0)); // close reading

try {
var_dump($socket->shutdown(1)); // close writing
} catch(Exception $exception) { var_dump('not connected'); }

$socket->close();
?>
--EXPECTF--
Warning: Socket::shutdown() expects exactly 1 parameter, 0 given in %s on line %d
bool(false)
bool(true)
string(13) "not connected"