Merge remote-tracking branch 'upstream/master' into pr3

.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

CMakeLists.txt

@@ -50,13 +50,18 @@ 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}")
+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)
@@ -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")

README.md

@@ -320,36 +320,94 @@ 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 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
+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.
 
 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 thes 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.
@@ -549,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
     */

net.c

@@ -277,12 +277,28 @@ 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;
+        socklen_t optlen = sizeof(so_error);
+        fail = getsockopt(c->fd, SOL_SOCKET, SO_ERROR, &so_error, &optlen);
+        if (fail == 0) {
+            if (so_error == 0) {
+                /* Socket 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;

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;
 }

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 <<EOF
 daemonize yes
+${ENABLE_DEBUG_CMD}
 pidfile ${PID_FILE}
 port ${REDIS_PORT}
 bind 127.0.0.1