Skip to content

Commit 12aa64c

Browse files
matyhtfmatyhtf
authored
fix sync-client async connect bugs (swoole#4152)
* fix sync-client async connect bugs * fix core-tests * fix
1 parent 51a76b4 commit 12aa64c

File tree

6 files changed

+119
-24
lines changed

6 files changed

+119
-24
lines changed

‎ext-src/swoole_client.cc

Lines changed: 25 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -139,15 +139,26 @@ static int client_select_wait(zval *sock_array, fd_set *fds);
139139

140140
static sw_inline Client *client_get_ptr(zval *zobject) {
141141
Client *cli = php_swoole_client_get_cli(zobject);
142-
if (cli && cli->socket && cli->active == 1) {
143-
return cli;
144-
} else {
145-
swoole_set_last_error(SW_ERROR_CLIENT_NO_CONNECTION);
146-
zend_update_property_long(
147-
swoole_client_ce, SW_Z8_OBJ_P(zobject), ZEND_STRL("errCode"), swoole_get_last_error());
148-
php_swoole_error(E_WARNING, "client is not connected to server");
149-
return nullptr;
142+
if (cli && cli->socket) {
143+
if (cli->active) {
144+
return cli;
145+
}
146+
if (cli->async_connect) {
147+
cli->async_connect = false;
148+
int error = -1;
149+
cli->get_socket()->get_option(SOL_SOCKET, SO_ERROR, &error);
150+
if (error == 0) {
151+
cli->active = 1;
152+
return cli;
153+
}
154+
php_swoole_client_free(zobject, cli);
155+
}
150156
}
157+
swoole_set_last_error(SW_ERROR_CLIENT_NO_CONNECTION);
158+
zend_update_property_long(
159+
swoole_client_ce, SW_Z8_OBJ_P(zobject), ZEND_STRL("errCode"), swoole_get_last_error());
160+
php_swoole_error(E_WARNING, "client is not connected to server");
161+
return nullptr;
151162
}
152163

153164
// clang-format off
@@ -809,16 +820,19 @@ static PHP_METHOD(swoole_client, connect) {
809820
}
810821
}
811822

812-
// nonblock async
813823
if (cli->connect(cli, host, port, timeout, sock_flag) < 0) {
824+
zend_update_property_long(
825+
swoole_client_ce, SW_Z8_OBJ_P(ZEND_THIS), ZEND_STRL("errCode"), swoole_get_last_error());
826+
// async connect
827+
if (cli->async_connect) {
828+
RETURN_TRUE;
829+
}
814830
php_swoole_error(E_WARNING,
815831
"connect to server[%s:%d] failed. Error: %s[%d]",
816832
host,
817833
(int) port,
818834
swoole_strerror(swoole_get_last_error()),
819835
swoole_get_last_error());
820-
zend_update_property_long(
821-
swoole_client_ce, SW_Z8_OBJ_P(ZEND_THIS), ZEND_STRL("errCode"), swoole_get_last_error());
822836
php_swoole_client_free(ZEND_THIS, cli);
823837
RETURN_FALSE;
824838
}

‎ext-src/swoole_event.cc

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -497,7 +497,7 @@ static PHP_FUNCTION(swoole_event_add) {
497497

498498
if (swoole_event_add(socket, events) < 0) {
499499
php_swoole_fatal_error(E_WARNING, "swoole_event_add failed");
500-
efree(socket);
500+
socket->free();
501501
event_object_free(peo);
502502
RETURN_FALSE;
503503
}
@@ -579,12 +579,14 @@ static PHP_FUNCTION(swoole_event_set) {
579579
if (reactor_fd->fci_cache_read.function_handler) {
580580
sw_zend_fci_cache_discard(&reactor_fd->fci_cache_read);
581581
}
582+
sw_zend_fci_cache_persist(&fci_cache_read);
582583
reactor_fd->fci_cache_read = fci_cache_read;
583584
}
584585
if (fci_write.size != 0) {
585586
if (reactor_fd->fci_cache_write.function_handler) {
586587
sw_zend_fci_cache_discard(&reactor_fd->fci_cache_write);
587588
}
589+
sw_zend_fci_cache_persist(&fci_cache_write);
588590
reactor_fd->fci_cache_write = fci_cache_write;
589591
}
590592

‎include/swoole_client.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ class Client {
4949
bool remove_delay = false;
5050
bool closed = false;
5151
bool high_watermark = false;
52+
bool async_connect = false;
5253

5354
/**
5455
* one package: length check
@@ -126,6 +127,10 @@ class Client {
126127
http_proxy->proxy_port = port;
127128
}
128129

130+
Socket *get_socket() {
131+
return socket;
132+
}
133+
129134
int sleep();
130135
int wakeup();
131136
int shutdown(int __how);

‎src/network/client.cc

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -546,13 +546,16 @@ static int Client_tcp_connect_sync(Client *cli, const char *host, int port, doub
546546
}
547547
#else
548548
ret = cli->socket->connect(cli->server_addr);
549-
if (ret < 0 && errno == EINPROGRESS) {
550-
errno = ETIMEDOUT;
551-
}
552549
#endif
553550
if (ret < 0) {
554551
if (errno == EINTR) {
555552
continue;
553+
} else if (errno == EINPROGRESS) {
554+
if (nonblock) {
555+
cli->async_connect = true;
556+
} else {
557+
errno = ETIMEDOUT;
558+
}
556559
}
557560
swoole_set_last_error(errno);
558561
}
@@ -575,23 +578,20 @@ static int Client_tcp_connect_sync(Client *cli, const char *host, int port, doub
575578
if (n > 0) {
576579
if (cli->socks5_handshake(buf, n) < 0) {
577580
return SW_ERR;
581+
}
582+
if (cli->socks5_proxy->state == SW_SOCKS5_STATE_READY) {
583+
break;
578584
} else {
579-
if (cli->socks5_proxy->state == SW_SOCKS5_STATE_READY) {
580-
break;
581-
} else {
582-
continue;
583-
}
585+
continue;
584586
}
585587
}
586588
return SW_ERR;
587589
}
588590
}
589591

590592
#ifdef SW_USE_OPENSSL
591-
if (cli->open_ssl) {
592-
if (cli->ssl_handshake() < 0) {
593-
return SW_ERR;
594-
}
593+
if (cli->open_ssl && cli->ssl_handshake() < 0) {
594+
return SW_ERR;
595595
}
596596
#endif
597597
}

‎tests/swoole_event/sync_client_1.phpt

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
--TEST--
2+
swoole_event: sync client
3+
--SKIPIF--
4+
<?php require __DIR__ . '/../include/skipif.inc';
5+
skip_if_offline();
6+
?>
7+
--FILE--
8+
<?php
9+
require __DIR__ . '/../include/bootstrap.php';
10+
11+
use Swoole\Client;
12+
use Swoole\Event;
13+
14+
swoole_async_set(['enable_coroutine' => false]);
15+
16+
$fp = new Client(SWOOLE_SOCK_TCP);
17+
18+
// sync connect
19+
$fp->connect('www.qq.com', 80);
20+
21+
Event::add($fp, function($fp) {
22+
$resp = $fp->recv(8192);
23+
Assert::contains($resp, 'Location: https://www.qq.com/');
24+
25+
Event::del($fp);
26+
$fp->close();
27+
28+
echo "Done\n";
29+
});
30+
31+
Event::write($fp, "GET / HTTP/1.1\r\nHost: www.qq.com\r\n\r\n");
32+
Event::wait();
33+
?>
34+
--EXPECT--
35+
Done

‎tests/swoole_event/sync_client_2.phpt

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
--TEST--
2+
swoole_event: sync client
3+
--SKIPIF--
4+
<?php require __DIR__ . '/../include/skipif.inc';
5+
skip_if_offline();
6+
?>
7+
--FILE--
8+
<?php
9+
require __DIR__ . '/../include/bootstrap.php';
10+
11+
use Swoole\Client;
12+
use Swoole\Event;
13+
14+
swoole_async_set(['enable_coroutine' => false]);
15+
16+
$fp = new Client(SWOOLE_SOCK_TCP);
17+
// async connect
18+
$result = $fp->connect('www.qq.com', 80, 1, 1);
19+
20+
Assert::true($result);
21+
Assert::eq($fp->errCode, SOCKET_EINPROGRESS);
22+
23+
Event::add($fp, null, function (Client $fp) {
24+
$fp->send("GET / HTTP/1.1\r\nHost: www.qq.com\r\n\r\n");
25+
Event::set($fp, function ($fp) {
26+
$resp = $fp->recv(8192);
27+
Assert::contains($resp, 'Location: https://www.qq.com/');
28+
29+
Event::del($fp);
30+
$fp->close();
31+
32+
echo "Done\n";
33+
}, null, SWOOLE_EVENT_READ);
34+
}, SWOOLE_EVENT_WRITE);
35+
36+
Event::wait();
37+
?>
38+
--EXPECT--
39+
Done

0 commit comments

Comments
 (0)