Skip to content

Fix network connect poll interuption handling #16606

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed

Fix network connect poll interuption handling #16606

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
34edb29
Fix network connect poll interuption handling
bukka Oct 25, 2024
687b335
Add unit test setup with php_network_connect_socket test
bukka Nov 28, 2024
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
108 changes: 71 additions & 37 deletions main/network.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,35 @@ typedef int php_non_blocking_flags_t;
fcntl(sock, F_SETFL, save)
#endif

#if HAVE_GETTIMEOFDAY
/* Subtract times */
static inline void sub_times(struct timeval a, struct timeval b, struct timeval *result)
{
result->tv_usec = a.tv_usec - b.tv_usec;
if (result->tv_usec < 0L) {
a.tv_sec--;
result->tv_usec += 1000000L;
}
result->tv_sec = a.tv_sec - b.tv_sec;
if (result->tv_sec < 0L) {
result->tv_sec++;
result->tv_usec -= 1000000L;
}
}

static inline void php_network_set_limit_time(struct timeval *limit_time,
struct timeval *timeout)
{
gettimeofday(limit_time, NULL);
limit_time->tv_sec += timeout->tv_sec;
limit_time->tv_usec += timeout->tv_usec;
if (limit_time->tv_usec >= 1000000) {
limit_time->tv_usec -= 1000000;
limit_time->tv_sec++;
}
}
#endif

/* Connect to a socket using an interruptible connect with optional timeout.
* Optionally, the connect can be made asynchronously, which will implicitly
* enable non-blocking mode on the socket.
Expand Down Expand Up @@ -351,25 +380,53 @@ PHPAPI int php_network_connect_socket(php_socket_t sockfd,
* expected when a connection is actively refused. This way,
* php_pollfd_for will return a mask with POLLOUT if the connection
* is successful and with POLLPRI otherwise. */
if ((n = php_pollfd_for(sockfd, POLLOUT|POLLPRI, timeout)) == 0) {
int events = POLLOUT|POLLPRI;
#else
if ((n = php_pollfd_for(sockfd, PHP_POLLREADABLE|POLLOUT, timeout)) == 0) {
int events = PHP_POLLREADABLE|POLLOUT;
#endif
struct timeval working_timeout;
#if HAVE_GETTIMEOFDAY
struct timeval limit_time, time_now;
#endif
if (timeout) {
memcpy(&working_timeout, timeout, sizeof(working_timeout));
#if HAVE_GETTIMEOFDAY
php_network_set_limit_time(&limit_time, &working_timeout);
#endif
error = PHP_TIMEOUT_ERROR_VALUE;
}

if (n > 0) {
len = sizeof(error);
/*
BSD-derived systems set errno correctly
Solaris returns -1 from getsockopt in case of error
*/
if (getsockopt(sockfd, SOL_SOCKET, SO_ERROR, (char*)&error, &len) != 0) {
while (true) {
n = php_pollfd_for(sockfd, events, timeout ? &working_timeout : NULL);
if (n < 0) {
if (errno == EINTR) {
#if HAVE_GETTIMEOFDAY
if (timeout) {
gettimeofday(&time_now, NULL);

if (!timercmp(&time_now, &limit_time, <)) {
/* time limit expired; no need for another poll */
error = PHP_TIMEOUT_ERROR_VALUE;
break;
} else {
/* work out remaining time */
sub_times(limit_time, time_now, &working_timeout);
}
}
#endif
continue;
}
ret = -1;
} else if (n == 0) {
error = PHP_TIMEOUT_ERROR_VALUE;
} else {
len = sizeof(error);
/* BSD-derived systems set errno correctly.
* Solaris returns -1 from getsockopt in case of error. */
if (getsockopt(sockfd, SOL_SOCKET, SO_ERROR, (char*)&error, &len) != 0) {
ret = -1;
}
}
} else {
/* whoops: sockfd has disappeared */
ret = -1;
break;
}

ok:
Expand All @@ -392,22 +449,6 @@ PHPAPI int php_network_connect_socket(php_socket_t sockfd,
}
/* }}} */

/* {{{ sub_times */
static inline void sub_times(struct timeval a, struct timeval b, struct timeval *result)
{
result->tv_usec = a.tv_usec - b.tv_usec;
if (result->tv_usec < 0L) {
a.tv_sec--;
result->tv_usec += 1000000L;
}
result->tv_sec = a.tv_sec - b.tv_sec;
if (result->tv_sec < 0L) {
result->tv_sec++;
result->tv_usec -= 1000000L;
}
}
/* }}} */

/* Bind to a local IP address.
* Returns the bound socket, or -1 on failure.
* */
Expand Down Expand Up @@ -777,7 +818,6 @@ PHPAPI php_socket_t php_network_accept_incoming(php_socket_t srvsock,
}
/* }}} */


/* Connect to a remote host using an interruptible connect with optional timeout.
* Optionally, the connect can be made asynchronously, which will implicitly
* enable non-blocking mode on the socket.
Expand Down Expand Up @@ -809,13 +849,7 @@ php_socket_t php_network_connect_socket_to_host(const char *host, unsigned short
if (timeout) {
memcpy(&working_timeout, timeout, sizeof(working_timeout));
#if HAVE_GETTIMEOFDAY
gettimeofday(&limit_time, NULL);
limit_time.tv_sec += working_timeout.tv_sec;
limit_time.tv_usec += working_timeout.tv_usec;
if (limit_time.tv_usec >= 1000000) {
limit_time.tv_usec -= 1000000;
limit_time.tv_sec++;
}
php_network_set_limit_time(&limit_time, &working_timeout);
#endif
}

Expand Down
31 changes: 31 additions & 0 deletions tests/unit/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
CC = gcc
CFLAGS = -g -Wall -I../../ -I../../Zend -I../../main -I../../TSRM -I. -I..
COMMON_LDFLAGS = ../../.libs/libphp.a -lcmocka -lpthread -lm -ldl -lresolv -lutil

TESTS = main/test_network
main/test_network_SRC = main/test_network.c
main/test_network_LDFLAGS = $(COMMON_LDFLAGS) -Wl,--wrap=connect,--wrap=poll,--wrap=getsockopt,--wrap=gettimeofday


# Build all tests
all: $(TESTS)

# Build rule for each test
$(TESTS):
$(CC) $(CFLAGS) -o [email protected] $($(basename $@)_SRC) $($(basename $@)_LDFLAGS)

# Run all tests
.PHONY: test
test: $(TESTS)
@echo "Running all tests..."
@for test in $(TESTS); do \
echo "Running $$test..."; \
$$test.out || exit 1; \
done

# Clean tests
.PHONY: clean
clean:
@for test in $(TESTS); do \
rm -f $$test.out; \
done
Loading
Loading