From ff57c18b9e7bcea564dc163eb195aefce44a12a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kristj=C3=A1n=20Valur=20J=C3=B3nsson?= Date: Thu, 24 Mar 2022 12:52:15 +0000 Subject: [PATCH 1/9] Embed debug information in windows static lib, rather than create a .pdb file Using .pdb files with .lib files on windows is very inconvenient, particularly if the .lib file is then linked as part of a different .dll. Chances are that the original .pdb will not be picked up or distributed along with the tooling. --- CMakeLists.txt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index fe6720b28..44829e3c1 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -53,10 +53,10 @@ ADD_LIBRARY(hiredis::hiredis_static ALIAS hiredis_static) SET_TARGET_PROPERTIES(hiredis PROPERTIES WINDOWS_EXPORT_ALL_SYMBOLS TRUE VERSION "${HIREDIS_SONAME}") +IF(WIN32) SET_TARGET_PROPERTIES(hiredis_static - PROPERTIES COMPILE_PDB_NAME hiredis_static) -SET_TARGET_PROPERTIES(hiredis_static - PROPERTIES COMPILE_PDB_NAME_DEBUG hiredis_static${CMAKE_DEBUG_POSTFIX}) + PROPERTIES COMPILE_FLAGS /Z7) +ENDIF() IF(WIN32 OR MINGW) TARGET_LINK_LIBRARIES(hiredis PUBLIC ws2_32 crypt32) TARGET_LINK_LIBRARIES(hiredis_static PUBLIC ws2_32 crypt32) From dd4bf97836d07df0a4502b72f9024d0231156711 Mon Sep 17 00:00:00 2001 From: Orgad Shaneh Date: Tue, 29 Mar 2022 17:16:19 +0300 Subject: [PATCH 2/9] Use the same name for static and shared libraries On all system except MSVC, the targets are different. Unix: libhiredis.so, libhiredis.a MinGW: libhiredis.dll+libhiredis.dll.a, libhiredis.a MSVC: hiredis.dll+hiredis.lib, hiredis_static.lib --- CMakeLists.txt | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index fe6720b28..899cd14d0 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -50,6 +50,11 @@ ADD_LIBRARY(hiredis_static STATIC ${hiredis_sources}) ADD_LIBRARY(hiredis::hiredis ALIAS hiredis) ADD_LIBRARY(hiredis::hiredis_static ALIAS hiredis_static) +IF(NOT MSVC) + SET_TARGET_PROPERTIES(hiredis_static + PROPERTIES OUTPUT_NAME hiredis) +ENDIF() + SET_TARGET_PROPERTIES(hiredis PROPERTIES WINDOWS_EXPORT_ALL_SYMBOLS TRUE VERSION "${HIREDIS_SONAME}") @@ -160,6 +165,10 @@ IF(ENABLE_SSL) ${hiredis_ssl_sources}) ADD_LIBRARY(hiredis_ssl_static STATIC ${hiredis_ssl_sources}) + IF(NOT MSVC) + SET_TARGET_PROPERTIES(hiredis_ssl_static + PROPERTIES OUTPUT_NAME hiredis_ssl) + ENDIF() IF (APPLE) SET_PROPERTY(TARGET hiredis_ssl PROPERTY LINK_FLAGS "-Wl,-undefined -Wl,dynamic_lookup") From f4b6ed28989ba0f6c59c64d015a8b0c830b3f1ca Mon Sep 17 00:00:00 2001 From: michael-grunder Date: Wed, 4 May 2022 13:13:54 -0700 Subject: [PATCH 3/9] Fix tests so they work for Redis 7.0 * Redis >= 7.0.0 disables the `DEBUG` command by default, which we need for our unit tests. * Downgrade to Redis 6.2.x in macOS temporarily There is a macOS specific TLS error on large payloads when running against 7.x.x so temporarily run our tests against 6.2, while we investigate the root cause. --- .github/workflows/build.yml | 3 ++- test.sh | 9 +++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index a0716289b..7859ff438 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -152,7 +152,8 @@ jobs: - name: Install dependencies run: | - brew install openssl redis + brew install openssl redis@6.2 + brew link redis@6.2 --force - name: Build hiredis run: USE_SSL=1 make diff --git a/test.sh b/test.sh index c72bcb0dc..a518db71f 100755 --- a/test.sh +++ b/test.sh @@ -5,9 +5,16 @@ REDIS_PORT=${REDIS_PORT:-56379} REDIS_SSL_PORT=${REDIS_SSL_PORT:-56443} TEST_SSL=${TEST_SSL:-0} SKIPS_AS_FAILS=${SKIPS_AS_FAILS-:0} +ENABLE_DEBUG_CMD= SSL_TEST_ARGS= SKIPS_ARG= +# We need to enable the DEBUG command for redis-server >= 7.0.0 +REDIS_MAJOR_VERSION="$(redis-server --version|awk -F'[^0-9]+' '{ print $2 }')" +if [ "$REDIS_MAJOR_VERSION" -gt "6" ]; then + ENABLE_DEBUG_CMD="enable-debug-command local" +fi + tmpdir=$(mktemp -d) PID_FILE=${tmpdir}/hiredis-test-redis.pid SOCK_FILE=${tmpdir}/hiredis-test-redis.sock @@ -49,8 +56,10 @@ cleanup() { } trap cleanup INT TERM EXIT + cat > ${tmpdir}/redis.conf < Date: Fri, 26 Mar 2021 14:40:32 +0000 Subject: [PATCH 4/9] Support failed async connects on windows. --- net.c | 21 +++++++++++++++++++-- sockcompat.c | 19 +++++++++++++++++-- 2 files changed, 36 insertions(+), 4 deletions(-) diff --git a/net.c b/net.c index c6b0e5d8e..d9e6d7a1c 100644 --- a/net.c +++ b/net.c @@ -277,12 +277,29 @@ int redisCheckConnectDone(redisContext *c, int *completed) { *completed = 1; return REDIS_OK; } - switch (errno) { + int error = errno; + if (error == EINPROGRESS) + { + /* must check error to see if connect failed. Get the socket error */ + int fail, so_error, optlen; + optlen = sizeof(so_error); + fail = getsockopt(c->fd, SOL_SOCKET, SO_ERROR, &so_error, &optlen); + if (fail == 0) { + if (so_error == 0) { + /* ocket is connected! */ + *completed = 1; + return REDIS_OK; + } + /* connection error; */ + errno = so_error; + error = so_error; + } + } + switch (error) { case EISCONN: *completed = 1; return REDIS_OK; case EALREADY: - case EINPROGRESS: case EWOULDBLOCK: *completed = 0; return REDIS_OK; diff --git a/sockcompat.c b/sockcompat.c index f99d14b05..31df32556 100644 --- a/sockcompat.c +++ b/sockcompat.c @@ -180,10 +180,17 @@ int win32_connect(SOCKET sockfd, const struct sockaddr *addr, socklen_t addrlen) /* For Winsock connect(), the WSAEWOULDBLOCK error means the same thing as * EINPROGRESS for POSIX connect(), so we do that translation to keep POSIX - * logic consistent. */ - if (errno == EWOULDBLOCK) { + * logic consistent. + * Additionally, WSAALREADY is can be reported as WSAEINVAL to and this is + * translated to EIO. Convert appropriately + */ + int err = errno; + if (err == EWOULDBLOCK) { errno = EINPROGRESS; } + else if (err == EIO) { + errno = EALREADY; + } return ret != SOCKET_ERROR ? ret : -1; } @@ -205,6 +212,14 @@ int win32_getsockopt(SOCKET sockfd, int level, int optname, void *optval, sockle } else { ret = getsockopt(sockfd, level, optname, (char*)optval, optlen); } + if (ret != SOCKET_ERROR && level == SOL_SOCKET && optname == SO_ERROR) { + /* translate SO_ERROR codes, if non-zero */ + int err = *(int*)optval; + if (err != 0) { + err = _wsaErrorToErrno(err); + *(int*)optval = err; + } + } _updateErrno(ret != SOCKET_ERROR); return ret != SOCKET_ERROR ? ret : -1; } From 94c1985bde0751fabc31deba437ffa6f6d52b696 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kristj=C3=A1n=20Valur=20J=C3=B3nsson?= Date: Thu, 8 Apr 2021 10:03:22 +0000 Subject: [PATCH 5/9] Use correct type for getsockopt() --- net.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/net.c b/net.c index d9e6d7a1c..d252555ff 100644 --- a/net.c +++ b/net.c @@ -281,8 +281,8 @@ int redisCheckConnectDone(redisContext *c, int *completed) { if (error == EINPROGRESS) { /* must check error to see if connect failed. Get the socket error */ - int fail, so_error, optlen; - optlen = sizeof(so_error); + int fail, so_error; + socklen_t optlen = sizeof(so_error); fail = getsockopt(c->fd, SOL_SOCKET, SO_ERROR, &so_error, &optlen); if (fail == 0) { if (so_error == 0) { From f246ee433d7d46b8132781759c660f9cb70b417d Mon Sep 17 00:00:00 2001 From: michael-grunder Date: Sun, 26 Jun 2022 14:14:58 -0700 Subject: [PATCH 6/9] Whitespace, style --- net.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/net.c b/net.c index d252555ff..3ff89edad 100644 --- a/net.c +++ b/net.c @@ -278,20 +278,19 @@ int redisCheckConnectDone(redisContext *c, int *completed) { return REDIS_OK; } int error = errno; - if (error == EINPROGRESS) - { + if (error == EINPROGRESS) { /* must check error to see if connect failed. Get the socket error */ int fail, so_error; socklen_t optlen = sizeof(so_error); fail = getsockopt(c->fd, SOL_SOCKET, SO_ERROR, &so_error, &optlen); if (fail == 0) { if (so_error == 0) { - /* ocket is connected! */ + /* Socket is connected! */ *completed = 1; return REDIS_OK; } /* connection error; */ - errno = so_error; + errno = so_error; error = so_error; } } From 47b57aa243120c9678bb1a01d6a9a3da70894d6f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kristj=C3=A1n=20Valur=20J=C3=B3nsson?= Date: Wed, 31 Mar 2021 09:24:44 +0000 Subject: [PATCH 7/9] Add some documentation on connect/disconnect callbacks and command callbacks --- README.md | 93 +++++++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 84 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index ed66220c7..a99776cc6 100644 --- a/README.md +++ b/README.md @@ -320,23 +320,48 @@ Redis. It returns a pointer to the newly created `redisAsyncContext` struct. The should be checked after creation to see if there were errors creating the connection. Because the connection that will be created is non-blocking, the kernel is not able to instantly return if the specified host and port is able to accept a connection. +In case of error, it is the caller's responsibility to free the context using `redisAsyncFree()` *Note: A `redisAsyncContext` is not thread-safe.* +An application function creating a connection might look like this: + ```c -redisAsyncContext *c = redisAsyncConnect("127.0.0.1", 6379); -if (c->err) { - printf("Error: %s\n", c->errstr); - // handle error +void appConnect(myAppData *appData) +{ + redisAsyncContext *c = redisAsyncConnect("127.0.0.1", 6379); + if (c->err) { + printf("Error: %s\n", c->errstr); + // handle error + redisAsyncFree(c); + c = NULL; + } else { + appData->context = c; + appData->connecting = 1; + c->data = appData; /* store application pointer for the callbacks */ + redisAsyncSetConnectCallback(c, appOnConnect); + redisAsyncSetDisconnectCallback(c, appOnDisconnect); + } } + ``` -The asynchronous context can hold a disconnect callback function that is called when the -connection is disconnected (either because of an error or per user request). This function should + +The asynchronous context _should_ hold a *connect* callback function that is called when the connection +attempt completes, either successfully or with an error. +It _can_ also hold a *disconnect* callback function that is called when the +connection is disconnected (either because of an error or per user request). Both callbacks should have the following prototype: ```c void(const redisAsyncContext *c, int status); ``` + +On a *connect*, the `status` argument is set to `REDIS_OK` if the connection attempted. In this +case, the context is ready to accept commands. If it is called with `REDIS_ERR` then the +connection attempt failed. The `err` field in the context can be accessed to find out the cause of the error. +After a failed connection attempt, the context object is automatically freed by the libary after calling +the connect callback. This may be a good point to create a new context and retry the connection. + On a disconnect, the `status` argument is set to `REDIS_OK` when disconnection was initiated by the user, or `REDIS_ERR` when the disconnection was caused by an error. When it is `REDIS_ERR`, the `err` field in the context can be accessed to find out the cause of the error. @@ -344,12 +369,45 @@ field in the context can be accessed to find out the cause of the error. The context object is always freed after the disconnect callback fired. When a reconnect is needed, the disconnect callback is a good point to do so. -Setting the disconnect callback can only be done once per context. For subsequent calls it will -return `REDIS_ERR`. The function to set the disconnect callback has the following prototype: +Setting the connect or disconnect callbacks can only be done once per context. For subsequent calls the +api will return `REDIS_ERR`. The function to set the callbacks have the following prototype: ```c +int redisAsyncSetConnectCallback(redisAsyncContext *ac, redisConnectCallback *fn); int redisAsyncSetDisconnectCallback(redisAsyncContext *ac, redisDisconnectCallback *fn); ``` -`ac->data` may be used to pass user data to this callback, the same can be done for redisConnectCallback. +`ac->data` may be used to pass user data to both of these this callbacks. An typical implementation +might look something like this: +```c +void appOnConnect(redisAsyncContext *c, int status) +{ + myAppData *appData = (myAppData*)c->data; /* get my application specific context*/ + appData->connecting = 0; + if (status == REDIS_OK) { + appData->connected = 1; + } else { + appData->connected = 0; + appData->err = c->err; + appData->context = NULL; /* avoid stale pointer when callback returns */ + } + appAttemptReconnect(); +} + +void appOnDisconnect(redisAsyncContext *c, int status) +{ + myAppData *appData = (myAppData*)c->data; /* get my application specific context*/ + appData->connected = 0; + appData->err = c->err; + appData->context = NULL; /* avoid stale pointer when callback returns */ + if (status == REDIS_OK) { + appNotifyDisconnectCompleted(mydata); + } else { + appNotifyUnexpectedDisconnect(mydata); + appAttemptReconnect(); + } +} +``` + + ### Sending commands and their callbacks In an asynchronous context, commands are automatically pipelined due to the nature of an event loop. @@ -382,6 +440,14 @@ valid for the duration of the callback. All pending callbacks are called with a `NULL` reply when the context encountered an error. +For every command issued, with the exception of **SUBSCRIBE** and **PSUBSCRIBE**, the callback is +called exactly once. Even if the context object id disconnected or deleted, every pending callback +will be called with a `NULL` reply. + +For **SUBSCRIBE** and **PSUBSCRIBE**, the callbacks may be called repeatedly until a `unsubscribe` +message arrives. This will be the last invocation of the callback. In case of error, the callbacks +may reive a final `NULL` reply instead. + ### Disconnecting An asynchronous connection can be terminated using: @@ -394,6 +460,15 @@ have been written to the socket, their respective replies have been read and the callbacks have been executed. After this, the disconnection callback is executed with the `REDIS_OK` status and the context object is freed. +The connection can be forcefully disconnected using +```c +void redisAsyncFree(redisAsyncContext *ac); +``` +In this case, nothing more is written to the socket, all pending callbacks are called with a `NULL` +reply and the disconnection callback is called with `REDIS_OK`, after which the context object +is freed. + + ### Hooking it up to event library *X* There are a few hooks that need to be set on the context object after it is created. From 1343988cee4b774d19864be9c9134fbaf6daf09b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kristj=C3=A1n=20Valur=20J=C3=B3nsson?= Date: Fri, 24 Jun 2022 17:05:19 +0000 Subject: [PATCH 8/9] Fix typos --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index a99776cc6..c0cd902ce 100644 --- a/README.md +++ b/README.md @@ -356,7 +356,7 @@ have the following prototype: void(const redisAsyncContext *c, int status); ``` -On a *connect*, the `status` argument is set to `REDIS_OK` if the connection attempted. In this +On a *connect*, the `status` argument is set to `REDIS_OK` if the connection attempt succeeded. In this case, the context is ready to accept commands. If it is called with `REDIS_ERR` then the connection attempt failed. The `err` field in the context can be accessed to find out the cause of the error. After a failed connection attempt, the context object is automatically freed by the libary after calling @@ -375,7 +375,7 @@ api will return `REDIS_ERR`. The function to set the callbacks have the followin int redisAsyncSetConnectCallback(redisAsyncContext *ac, redisConnectCallback *fn); int redisAsyncSetDisconnectCallback(redisAsyncContext *ac, redisDisconnectCallback *fn); ``` -`ac->data` may be used to pass user data to both of these this callbacks. An typical implementation +`ac->data` may be used to pass user data to both of thes callbacks. An typical implementation might look something like this: ```c void appOnConnect(redisAsyncContext *c, int status) From 2b115d56cd96e525dc169496853819f16099539e Mon Sep 17 00:00:00 2001 From: michael-grunder Date: Sun, 26 Jun 2022 14:42:31 -0700 Subject: [PATCH 9/9] Whitespace --- README.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index c0cd902ce..3907adddb 100644 --- a/README.md +++ b/README.md @@ -399,7 +399,7 @@ void appOnDisconnect(redisAsyncContext *c, int status) appData->err = c->err; appData->context = NULL; /* avoid stale pointer when callback returns */ if (status == REDIS_OK) { - appNotifyDisconnectCompleted(mydata); + appNotifyDisconnectCompleted(mydata); } else { appNotifyUnexpectedDisconnect(mydata); appAttemptReconnect(); @@ -624,9 +624,9 @@ ssl_context = redisCreateSSLContext( if(ssl_context == NULL || ssl_error != 0) { /* Handle error and abort... */ - /* e.g. - printf("SSL error: %s\n", - (ssl_error != 0) ? + /* e.g. + printf("SSL error: %s\n", + (ssl_error != 0) ? redisSSLContextGetError(ssl_error) : "Unknown error"); // Abort */