Skip to content

Commit bce124f

Browse files
committed
php#53: Fix socket closure order in both regular and SSL socket implementations
Apply deferred socket closure mechanism to prevent LibUV race conditions in both xp_socket.c and xp_ssl.c modules. Previously, closesocket() was called before poll_event cleanup, causing potential race conditions where LibUV operations could access already-closed socket descriptors. Changes: - xp_socket.c: Fix php_sockop_close() to transfer socket ownership to LibUV before disposal - xp_ssl.c: Apply same fix to php_openssl_sockop_close() with SSL-specific cleanup preserved - Move Windows async logic (shutdown/polling) before socket ownership transfer in both modules - When poll_event exists, transfer socket via poll_event->socket and ZEND_ASYNC_EVENT_SET_CLOSE_FD flag - Call poll_event dispose immediately after flag setting for proper cleanup order - libuv_reactor.c: Add libuv_close_poll_handle_cb() with cross-platform fd/socket closure - zend_async_API.h: Define ZEND_ASYNC_EVENT_F_CLOSE_FD flag for conditional descriptor closure This ensures LibUV completes all pending operations before socket closure across both socket implementations.
1 parent af6b7cd commit bce124f

File tree

3 files changed

+64
-20
lines changed

3 files changed

+64
-20
lines changed

Zend/zend_async_API.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -528,6 +528,7 @@ typedef struct {
528528

529529
// Flag indicating that the event has a zend_object reference by extra_offset.
530530
#define ZEND_ASYNC_EVENT_F_OBJ_REF (1u << 8) /* has zend_object ref */
531+
#define ZEND_ASYNC_EVENT_F_CLOSE_FD (1u << 9) /* close file descriptor after event cleanup */
531532

532533
#define ZEND_ASYNC_EVENT_REFERENCE_PREFIX ((uint32_t) 0x80) /* prefix for reference structures */
533534

@@ -574,6 +575,10 @@ typedef struct {
574575

575576
#define ZEND_ASYNC_EVENT_WITH_OBJECT_REF(ev) ((ev)->flags |= ZEND_ASYNC_EVENT_F_OBJ_REF)
576577

578+
#define ZEND_ASYNC_EVENT_SET_CLOSE_FD(ev) ((ev)->flags |= ZEND_ASYNC_EVENT_F_CLOSE_FD)
579+
#define ZEND_ASYNC_EVENT_CLR_CLOSE_FD(ev) ((ev)->flags &= ~ZEND_ASYNC_EVENT_F_CLOSE_FD)
580+
#define ZEND_ASYNC_EVENT_SHOULD_CLOSE_FD(ev) (((ev)->flags & ZEND_ASYNC_EVENT_F_CLOSE_FD) != 0)
581+
577582
// Convert awaitable Zend object to zend_async_event_t pointer
578583
#define ZEND_ASYNC_EVENT_IS_REFERENCE(ptr) \
579584
(*((const uint32_t *) (ptr)) == ZEND_ASYNC_EVENT_REFERENCE_PREFIX)

ext/openssl/xp_ssl.c

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2180,8 +2180,31 @@ static int php_openssl_sockop_close(php_stream *stream, int close_handle) /* {{{
21802180
} while (n == -1 && php_socket_errno() == EINTR);
21812181
}
21822182
#endif
2183-
closesocket(sslsock->s.socket);
2184-
sslsock->s.socket = SOCK_ERR;
2183+
/**
2184+
* If we are in an async context and there is an active EventLoop, we should not close the
2185+
* socket immediately, because there might be pending operations for this socket in the loop.
2186+
* Instead, we transfer the ownership of the descriptor to the EventLoop which will close it
2187+
* once all pending operations are finished.
2188+
*/
2189+
if (sslsock->s.poll_event) {
2190+
sslsock->s.poll_event->socket = sslsock->s.socket;
2191+
2192+
/* Set flag to close descriptor after EventLoop cleanup */
2193+
ZEND_ASYNC_EVENT_SET_CLOSE_FD(&sslsock->s.poll_event->base);
2194+
sslsock->s.socket = SOCK_ERR;
2195+
sslsock->s.poll_event->base.dispose(&sslsock->s.poll_event->base);
2196+
sslsock->s.poll_event = NULL;
2197+
} else {
2198+
/* Just close the socket ourselves immediately */
2199+
closesocket(sslsock->s.socket);
2200+
sslsock->s.socket = SOCK_ERR;
2201+
}
2202+
}
2203+
} else {
2204+
/* Cleanup async event handle before freeing other resources */
2205+
if (sslsock->s.poll_event) {
2206+
sslsock->s.poll_event->base.dispose(&sslsock->s.poll_event->base);
2207+
sslsock->s.poll_event = NULL;
21852208
}
21862209
}
21872210

@@ -2204,12 +2227,6 @@ static int php_openssl_sockop_close(php_stream *stream, int close_handle) /* {{{
22042227
pefree(sslsock->reneg, php_stream_is_persistent(stream));
22052228
}
22062229

2207-
/* Cleanup async event handle before freeing socket structure */
2208-
if (sslsock->s.poll_event) {
2209-
sslsock->s.poll_event->base.dispose(&sslsock->s.poll_event->base);
2210-
sslsock->s.poll_event = NULL;
2211-
}
2212-
22132230
pefree(sslsock, php_stream_is_persistent(stream));
22142231

22152232
return 0;

main/streams/xp_socket.c

Lines changed: 34 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -275,20 +275,42 @@ static int php_sockop_close(php_stream *stream, int close_handle)
275275
* We use a small timeout which should encourage the OS to send the data,
276276
* but at the same time avoid hanging indefinitely.
277277
* */
278-
do {
279-
n = php_pollfd_for_ms(sock->socket, POLLOUT, 500);
280-
} while (n == -1 && php_socket_errno() == EINTR);
278+
if (ZEND_ASYNC_IS_ACTIVE) {
279+
struct timeval tv = {0, 500000}; // 500ms
280+
network_async_await_stream_socket(stream, POLLOUT, &tv);
281+
} else {
282+
do {
283+
n = php_pollfd_for_ms(sock->socket, POLLOUT, 500);
284+
} while (n == -1 && php_socket_errno() == EINTR);
285+
}
281286
#endif
282-
closesocket(sock->socket);
283-
sock->socket = SOCK_ERR;
284-
}
285-
286-
}
287287

288-
/* Cleanup async event handle before freeing socket structure */
289-
if (sock->poll_event) {
290-
sock->poll_event->base.dispose(&sock->poll_event->base);
291-
sock->poll_event = NULL;
288+
/**
289+
* If we are in an async context and there is an active EventLoop, we should not close the
290+
* socket immediately, because there might be pending operations for this socket in the loop.
291+
* Instead, we transfer the ownership of the descriptor to the EventLoop which will close it
292+
* once all pending operations are finished.
293+
*/
294+
if (sock->poll_event) {
295+
sock->poll_event->socket = sock->socket;
296+
297+
/* Set flag to close descriptor after EventLoop leanup */
298+
ZEND_ASYNC_EVENT_SET_CLOSE_FD(&sock->poll_event->base);
299+
sock->socket = SOCK_ERR;
300+
sock->poll_event->base.dispose(&sock->poll_event->base);
301+
sock->poll_event = NULL;
302+
} else {
303+
// Just the socket close ourselves immediately
304+
closesocket(sock->socket);
305+
sock->socket = SOCK_ERR;
306+
}
307+
}
308+
} else {
309+
/* Cleanup async event handle before freeing socket structure */
310+
if (sock->poll_event) {
311+
sock->poll_event->base.dispose(&sock->poll_event->base);
312+
sock->poll_event = NULL;
313+
}
292314
}
293315

294316
pefree(sock, php_stream_is_persistent(stream));

0 commit comments

Comments
 (0)