Skip to content

Commit 62239db

Browse files
matyhtfmatyhtf
authored
Optimize Socket::close() (#4956)
* Optimize socket close * fix tests * optimize code * optimize code [2] * fix tests [3] * fix tests [4] * add test * fix * fix tests * fix tests * refactor * fix tests
1 parent bdfb227 commit 62239db

25 files changed

+797
-537
lines changed

‎core-tests/src/coroutine/hook.cpp

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -428,3 +428,52 @@ TEST(coroutine_hook, lseek) {
428428
swoole_coroutine_close(fd);
429429
ASSERT_EQ(offset, 0);
430430
}
431+
432+
extern std::pair<std::shared_ptr<Socket>, std::shared_ptr<Socket>> create_socket_pair();
433+
434+
TEST(coroutine_hook, socket_close) {
435+
coroutine::run([&](void *arg) {
436+
auto pair = create_socket_pair();
437+
438+
auto buffer = sw_tg_buffer();
439+
buffer->clear();
440+
buffer->append_random_bytes(256 * 1024, false);
441+
442+
std::map<std::string, bool> results;
443+
auto _sock = pair.first;
444+
auto _fd = _sock->move_fd();
445+
swoole_coroutine_socket_create(_fd);
446+
447+
// write co
448+
Coroutine::create([&](void *) {
449+
SW_LOOP_N(32) {
450+
ssize_t result = swoole_coroutine_write(_fd, buffer->value(), buffer->get_length());
451+
if (result < 0 && errno == ECANCELED) {
452+
ASSERT_EQ(swoole_coroutine_close(_fd), -1);
453+
ASSERT_EQ(errno, SW_ERROR_CO_SOCKET_CLOSE_WAIT);
454+
results["write"] = true;
455+
break;
456+
}
457+
}
458+
});
459+
460+
// read co
461+
Coroutine::create([&](void *) {
462+
SW_LOOP_N(32) {
463+
char buf[4096];
464+
ssize_t result = swoole_coroutine_read(_fd, buf, sizeof(buf));
465+
if (result < 0 && errno == ECANCELED) {
466+
ASSERT_EQ(swoole_coroutine_close(_fd), 0);
467+
results["read"] = true;
468+
break;
469+
}
470+
}
471+
});
472+
473+
System::sleep(0.1);
474+
ASSERT_EQ(swoole_coroutine_close(_fd), -1);
475+
ASSERT_EQ(errno, SW_ERROR_CO_SOCKET_CLOSE_WAIT);
476+
ASSERT_TRUE(results["write"]);
477+
ASSERT_TRUE(results["read"]);
478+
});
479+
}

‎core-tests/src/coroutine/socket.cpp

Lines changed: 100 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323

2424
using namespace swoole::test;
2525

26+
using swoole::Coroutine;
2627
using swoole::HttpProxy;
2728
using swoole::Protocol;
2829
using swoole::Socks5Proxy;
@@ -667,7 +668,7 @@ TEST(coroutine_socket, event_hup) {
667668
ASSERT_EQ(sock.errCode, 0);
668669

669670
auto buf = sock.get_read_buffer();
670-
swoole::Coroutine::create([&sock](void *args) {
671+
Coroutine::create([&sock](void *args) {
671672
System::sleep(0.01);
672673
sock.shutdown(SHUT_RDWR);
673674
});
@@ -762,7 +763,7 @@ TEST(coroutine_socket, write_and_read) {
762763
std::string text = "Hello World";
763764
size_t length = text.length();
764765

765-
swoole::Coroutine::create([&](void *) {
766+
Coroutine::create([&](void *) {
766767
Socket sock(pairs[0], SW_SOCK_UNIX_STREAM);
767768
ssize_t result = sock.write(text.c_str(), length);
768769
sock.close();
@@ -787,7 +788,7 @@ TEST(coroutine_socket, write_and_read_2) {
787788
std::string text = "Hello World";
788789
size_t length = text.length();
789790

790-
swoole::Coroutine::create([&](void *) {
791+
Coroutine::create([&](void *) {
791792
Socket sock(pairs[0], AF_UNIX, SOCK_STREAM, 0);
792793
ssize_t result = sock.write(text.c_str(), length);
793794
sock.close();
@@ -812,7 +813,7 @@ TEST(coroutine_socket, writev_and_readv) {
812813
size_t length = text.length();
813814
socketpair(AF_UNIX, SOCK_STREAM, 0, pairs);
814815

815-
swoole::Coroutine::create([&](void *) {
816+
Coroutine::create([&](void *) {
816817
std::unique_ptr<iovec[]> iov(new iovec[iovcnt]);
817818
for (int i = 0; i < iovcnt; i++) {
818819
iov[i].iov_base = (void *) text.c_str();
@@ -854,7 +855,7 @@ TEST(coroutine_socket, writevall_and_readvall) {
854855
size_t length = text.length();
855856
socketpair(AF_UNIX, SOCK_STREAM, 0, pairs);
856857

857-
swoole::Coroutine::create([&](void *) {
858+
Coroutine::create([&](void *) {
858859
std::unique_ptr<iovec[]> iov(new iovec[iovcnt]);
859860
for (int i = 0; i < iovcnt; i++) {
860861
iov[i].iov_base = (void *) text.c_str();
@@ -892,7 +893,7 @@ TEST(coroutine_socket, sendfile) {
892893
coroutine::run([&](void *arg) {
893894
int pairs[2];
894895
socketpair(AF_UNIX, SOCK_STREAM, 0, pairs);
895-
swoole::Coroutine::create([&](void *) {
896+
Coroutine::create([&](void *) {
896897
std::string file = get_jpg_file();
897898
Socket sock(pairs[0], SW_SOCK_UNIX_STREAM);
898899
bool result = sock.sendfile(file.c_str(), 0, 0);
@@ -1040,7 +1041,7 @@ TEST(coroutine_socket, peek) {
10401041
std::string text = "Hello World";
10411042
size_t length = text.length();
10421043

1043-
swoole::Coroutine::create([&](void *) {
1044+
Coroutine::create([&](void *) {
10441045
Socket sock(pairs[0], SW_SOCK_UNIX_STREAM);
10451046
ssize_t result = sock.write(text.c_str(), length);
10461047
sock.close();
@@ -1065,7 +1066,7 @@ TEST(coroutine_socket, sendmsg_and_recvmsg) {
10651066
std::string text = "Hello World";
10661067
size_t length = text.length();
10671068

1068-
swoole::Coroutine::create([&](void *) {
1069+
Coroutine::create([&](void *) {
10691070
Socket sock(pairs[0], SW_SOCK_UNIX_STREAM);
10701071
struct msghdr msg;
10711072
struct iovec ivec;
@@ -1108,3 +1109,94 @@ TEST(coroutine_socket, sendmsg_and_recvmsg) {
11081109
ASSERT_STREQ(buf, text.c_str());
11091110
});
11101111
}
1112+
1113+
std::pair<std::shared_ptr<Socket>, std::shared_ptr<Socket>> create_socket_pair() {
1114+
int pairs[2];
1115+
socketpair(AF_UNIX, SOCK_STREAM, 0, pairs);
1116+
1117+
auto sock0 = new Socket(pairs[0], SW_SOCK_UNIX_STREAM);
1118+
auto sock1 = new Socket(pairs[1], SW_SOCK_UNIX_STREAM);
1119+
1120+
sock0->get_socket()->set_buffer_size(65536);
1121+
sock1->get_socket()->set_buffer_size(65536);
1122+
1123+
std::pair<std::shared_ptr<Socket>, std::shared_ptr<Socket>> result(sock0, sock1);
1124+
return result;
1125+
}
1126+
1127+
TEST(coroutine_socket, close) {
1128+
coroutine::run([&](void *arg) {
1129+
auto pair = create_socket_pair();
1130+
1131+
auto buffer = sw_tg_buffer();
1132+
buffer->clear();
1133+
buffer->append_random_bytes(256 * 1024, false);
1134+
1135+
std::map<std::string, bool> results;
1136+
auto _sock = pair.first;
1137+
1138+
// write co
1139+
Coroutine::create([&](void *) {
1140+
SW_LOOP_N(32) {
1141+
ssize_t result = _sock->write(buffer->value(), buffer->get_length());
1142+
if (result < 0 && _sock->errCode == ECANCELED) {
1143+
ASSERT_FALSE(_sock->close());
1144+
ASSERT_EQ(_sock->errCode, SW_ERROR_CO_SOCKET_CLOSE_WAIT);
1145+
results["write"] = true;
1146+
ASSERT_EQ(_sock->write(buffer->value(), buffer->get_length()), -1);
1147+
ASSERT_EQ(_sock->errCode, EBADF);
1148+
break;
1149+
}
1150+
}
1151+
});
1152+
1153+
// read co
1154+
Coroutine::create([&](void *) {
1155+
SW_LOOP_N(32) {
1156+
char buf[4096];
1157+
ssize_t result = _sock->read(buf, sizeof(buf));
1158+
if (result < 0 && _sock->errCode == ECANCELED) {
1159+
ASSERT_TRUE(_sock->close());
1160+
results["read"] = true;
1161+
break;
1162+
}
1163+
}
1164+
});
1165+
1166+
System::sleep(0.1);
1167+
ASSERT_FALSE(_sock->close());
1168+
ASSERT_EQ(_sock->errCode, SW_ERROR_CO_SOCKET_CLOSE_WAIT);
1169+
ASSERT_TRUE(_sock->is_closed());
1170+
ASSERT_TRUE(results["write"]);
1171+
ASSERT_TRUE(results["read"]);
1172+
ASSERT_FALSE(_sock->close());
1173+
ASSERT_EQ(_sock->errCode, EBADF);
1174+
});
1175+
}
1176+
1177+
TEST(coroutine_socket, cancel) {
1178+
coroutine::run([&](void *arg) {
1179+
auto pair = create_socket_pair();
1180+
1181+
auto buffer = sw_tg_buffer();
1182+
buffer->clear();
1183+
buffer->append_random_bytes(256 * 1024, false);
1184+
1185+
std::map<std::string, bool> results;
1186+
// read co
1187+
Coroutine::create([&](void *) {
1188+
SW_LOOP_N(32) {
1189+
char buf[4096];
1190+
ssize_t result = pair.first->read(buf, sizeof(buf));
1191+
if (result < 0 && pair.first->errCode == ECANCELED) {
1192+
results["read"] = true;
1193+
break;
1194+
}
1195+
}
1196+
});
1197+
1198+
System::sleep(0.1);
1199+
pair.first->cancel(SW_EVENT_READ);
1200+
ASSERT_TRUE(results["read"]);
1201+
});
1202+
}

‎ext-src/php_swoole.cc

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -599,6 +599,7 @@ PHP_MINIT_FUNCTION(swoole) {
599599
SW_REGISTER_LONG_CONSTANT("SWOOLE_ERROR_CO_NOT_EXISTS", SW_ERROR_CO_NOT_EXISTS);
600600
SW_REGISTER_LONG_CONSTANT("SWOOLE_ERROR_CO_CANCELED", SW_ERROR_CO_CANCELED);
601601
SW_REGISTER_LONG_CONSTANT("SWOOLE_ERROR_CO_TIMEDOUT", SW_ERROR_CO_TIMEDOUT);
602+
SW_REGISTER_LONG_CONSTANT("SWOOLE_ERROR_CO_SOCKET_CLOSE_WAIT", SW_ERROR_CO_SOCKET_CLOSE_WAIT);
602603

603604
/**
604605
* trace log
@@ -633,6 +634,7 @@ PHP_MINIT_FUNCTION(swoole) {
633634
SW_REGISTER_LONG_CONSTANT("SWOOLE_TRACE_TABLE", SW_TRACE_TABLE);
634635
SW_REGISTER_LONG_CONSTANT("SWOOLE_TRACE_CO_CURL", SW_TRACE_CO_CURL);
635636
SW_REGISTER_LONG_CONSTANT("SWOOLE_TRACE_CARES", SW_TRACE_CARES);
637+
SW_REGISTER_LONG_CONSTANT("SWOOLE_TRACE_ZLIB", SW_TRACE_ZLIB);
636638
SW_REGISTER_LONG_CONSTANT("SWOOLE_TRACE_ALL", SW_TRACE_ALL);
637639

638640
/**

‎ext-src/php_swoole_cxx.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,8 @@ SW_API bool php_swoole_socket_is_closed(zval *zobject);
9696
SW_API bool php_swoole_socket_set_ssl(swoole::coroutine::Socket *sock, zval *zset);
9797
#endif
9898
SW_API bool php_swoole_socket_set_protocol(swoole::coroutine::Socket *sock, zval *zset);
99-
SW_API bool php_swoole_client_set(swoole::coroutine::Socket *cli, zval *zset);
99+
SW_API bool php_swoole_socket_set(swoole::coroutine::Socket *cli, zval *zset);
100+
#define php_swoole_client_set php_swoole_socket_set
100101
SW_API php_stream *php_swoole_create_stream_from_socket(php_socket_t _fd,
101102
int domain,
102103
int type,

‎ext-src/php_swoole_private.h

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,7 @@ enum php_swoole_hook_type {
150150
};
151151
//---------------------------------------------------------
152152

153-
static sw_inline enum swSocketType php_swoole_socktype(long type) {
153+
static sw_inline enum swSocketType php_swoole_get_socket_type(long type) {
154154
return (enum swSocketType)(type & (~SW_FLAG_SYNC) & (~SW_FLAG_ASYNC) & (~SW_FLAG_KEEP) & (~SW_SOCK_SSL));
155155
}
156156

@@ -651,7 +651,7 @@ static sw_inline void sw_zend_class_unset_property_deny(zend_object *object, zen
651651
std_object_handlers.unset_property(object, member, cache_slot);
652652
}
653653

654-
static sw_inline zval *sw_zend_read_property(zend_class_entry *ce, zval *obj, const char *s, int len, int silent) {
654+
static sw_inline zval *sw_zend_read_property(zend_class_entry *ce, zval *obj, const char *s, size_t len, int silent) {
655655
zval rv, *property = zend_read_property(ce, SW_Z8_OBJ_P(obj), s, len, silent, &rv);
656656
if (UNEXPECTED(property == &EG(uninitialized_zval))) {
657657
zend_update_property_null(ce, SW_Z8_OBJ_P(obj), s, len);
@@ -677,7 +677,7 @@ static sw_inline zval *sw_zend_read_property_ex(zend_class_entry *ce, zval *obj,
677677
}
678678

679679
static sw_inline zval *sw_zend_read_property_not_null(
680-
zend_class_entry *ce, zval *obj, const char *s, int len, int silent) {
680+
zend_class_entry *ce, zval *obj, const char *s, size_t len, int silent) {
681681
zval rv, *property = zend_read_property(ce, SW_Z8_OBJ_P(obj), s, len, silent, &rv);
682682
zend_uchar type = Z_TYPE_P(property);
683683
return (type == IS_NULL || UNEXPECTED(type == IS_UNDEF)) ? NULL : property;
@@ -689,7 +689,7 @@ static sw_inline zval *sw_zend_read_property_not_null_ex(zend_class_entry *ce, z
689689
return (type == IS_NULL || UNEXPECTED(type == IS_UNDEF)) ? NULL : property;
690690
}
691691

692-
static sw_inline zval *sw_zend_update_and_read_property_array(zend_class_entry *ce, zval *obj, const char *s, int len) {
692+
static sw_inline zval *sw_zend_update_and_read_property_array(zend_class_entry *ce, zval *obj, const char *s, size_t len) {
693693
zval ztmp;
694694
array_init(&ztmp);
695695
zend_update_property(ce, SW_Z8_OBJ_P(obj), s, len, &ztmp);
@@ -698,7 +698,7 @@ static sw_inline zval *sw_zend_update_and_read_property_array(zend_class_entry *
698698
}
699699

700700
static sw_inline zval *sw_zend_read_and_convert_property_array(
701-
zend_class_entry *ce, zval *obj, const char *s, int len, int silent) {
701+
zend_class_entry *ce, zval *obj, const char *s, size_t len, int silent) {
702702
zval rv, *property = zend_read_property(ce, SW_Z8_OBJ_P(obj), s, len, silent, &rv);
703703
if (Z_TYPE_P(property) != IS_ARRAY) {
704704
// NOTICE: if user unset the property, zend_read_property will return uninitialized_zval instead of NULL pointer
@@ -908,7 +908,7 @@ static sw_inline char *php_swoole_format_date(char *format, size_t format_len, t
908908
return return_str;
909909
}
910910

911-
static sw_inline char *php_swoole_url_encode(const char *value, size_t value_len, int *exten) {
911+
static sw_inline char *php_swoole_url_encode(const char *value, size_t value_len, size_t *exten) {
912912
zend_string *str = php_url_encode(value, value_len);
913913
*exten = ZSTR_LEN(str);
914914
char *return_str = estrndup(ZSTR_VAL(str), ZSTR_LEN(str));

‎ext-src/swoole_client.cc

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -584,7 +584,7 @@ static Client *php_swoole_client_new(zval *zobject, char *host, int host_len, in
584584
}
585585

586586
long type = Z_LVAL_P(ztype);
587-
int client_type = php_swoole_socktype(type);
587+
int client_type = php_swoole_get_socket_type(type);
588588
if ((client_type == SW_SOCK_TCP || client_type == SW_SOCK_TCP6) && (port <= 0 || port > SW_CLIENT_MAX_PORT)) {
589589
php_swoole_fatal_error(E_WARNING, "The port is invalid");
590590
swoole_set_last_error(SW_ERROR_INVALID_PARAMS);
@@ -623,7 +623,7 @@ static Client *php_swoole_client_new(zval *zobject, char *host, int host_len, in
623623
}
624624
} else {
625625
_create_socket:
626-
cli = new Client(php_swoole_socktype(type), false);
626+
cli = new Client(php_swoole_get_socket_type(type), false);
627627
if (cli->socket == nullptr) {
628628
php_swoole_sys_error(E_WARNING, "Client_create() failed");
629629
zend_update_property_long(Z_OBJCE_P(zobject), SW_Z8_OBJ_P(zobject), ZEND_STRL("errCode"), errno);
@@ -667,7 +667,7 @@ static PHP_METHOD(swoole_client, __construct) {
667667
RETURN_FALSE;
668668
}
669669

670-
int client_type = php_swoole_socktype(type);
670+
int client_type = php_swoole_get_socket_type(type);
671671
if (client_type < SW_SOCK_TCP || client_type > SW_SOCK_UNIX_DGRAM) {
672672
const char *space, *class_name = get_active_class_name(&space);
673673
zend_type_error("%s%s%s() expects parameter %d to be client type, unknown type " ZEND_LONG_FMT " given",

0 commit comments

Comments
 (0)