Check sockets from connection pool before using them and try to reconnect
them if they are not usable any longer.
* memcache/apr_memcache.c::ms_find_conn:
Check if the socket returned from the connection pool is still readable. If
not then invalidate the connection in the pool and request a new one
from the connection pool. Repeat this until a valid socket is returned
or this was done the maximum number of connections in the pool plus one.
This ensures that at least one new socket was created. If a new socket
does not work this indicates a broken backend and not just a restart in
the past. In this case return an error like previously.
* test/testmemcache.c:
Add new test for connection validation.
* test/memcachedmock.c:
For the new test we need a memcached mock server that we control and
can restart.
* test/testmemcache.h:
Shared defines between test/testmemcache.c and test/memcachedmock.c.
* test/Makefile.in:
* test/Makefile.win:
* test/NWGNUmakefile:
* test/NWGNUmemcachedmock:
Needed changes to build test/memcachedmock.c on different platforms.
git-svn-id: https://svn.apache.org/repos/asf/apr/apr/trunk@1909585 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/memcache/apr_memcache.c b/memcache/apr_memcache.c
index 52e29e1..5c028db 100644
--- a/memcache/apr_memcache.c
+++ b/memcache/apr_memcache.c
@@ -222,22 +222,83 @@
return NULL;
}
+
+/* Forward declare mc_conn_construct */
+static apr_status_t
+mc_conn_construct(void **conn_, void *params, apr_pool_t *pool);
+
static apr_status_t ms_find_conn(apr_memcache_server_t *ms, apr_memcache_conn_t **conn)
{
- apr_status_t rv;
+ apr_status_t rv = APR_SUCCESS;
apr_bucket_alloc_t *balloc;
apr_bucket *e;
+#if APR_HAS_THREADS
+ int i;
+#endif
+ int atreadeof;
#if APR_HAS_THREADS
- rv = apr_reslist_acquire(ms->conns, (void **)conn);
+ /*
+ * In order to avoid an endless loop in case that a freshly connected
+ * socket is immediately closed by the remote side we limit this loop
+ * to the maxium number of connections in the reslist plus 1.
+ */
+ for (i = 0; i <= ms->max; i++) {
+ rv = apr_reslist_acquire(ms->conns, (void **)conn);
+
+ if (rv != APR_SUCCESS) {
+ return rv;
+ }
+
+ atreadeof = 0;
+ rv = apr_socket_atreadeof((*conn)->sock, &atreadeof);
+
+ if ((rv == APR_SUCCESS) && !atreadeof) {
+ break;
+ }
+ /*
+ * The socket we got is fishy. But maybe the memcached was just
+ * restarted. Hence give it a chance by destroying the socket and
+ * getting a new one.
+ */
+ rv = apr_reslist_invalidate(ms->conns, *conn);
+ *conn = NULL;
+
+ if (rv != APR_SUCCESS) {
+ return rv;
+ }
+ }
+
+ /*
+ * Even after refreshing all sockets we still do not have a working one.
+ * Give up.
+ */
+ if (!(*conn)) {
+ return APR_EGENERAL;
+ }
#else
*conn = ms->conn;
- rv = APR_SUCCESS;
-#endif
-
- if (rv != APR_SUCCESS) {
- return rv;
+ atreadeof = 0;
+ if (*conn) {
+ rv = apr_socket_atreadeof((*conn)->sock, &atreadeof);
+ if ((rv != APR_SUCCESS) || !atreadeof) {
+ ms->conn = NULL;
+ apr_pool_destroy((*conn)->p);
+ rv = mc_conn_construct((void**)conn, ms, ms->p);
+ if (rv != APR_SUCCESS) {
+ return rv;
+ }
+ ms->conn = *conn;
+ }
}
+ else {
+ rv = mc_conn_construct((void**)conn, ms, ms->p);
+ if (rv != APR_SUCCESS) {
+ return rv;
+ }
+ ms->conn = *conn;
+ }
+#endif
balloc = apr_bucket_alloc_create((*conn)->tp);
(*conn)->bb = apr_brigade_create((*conn)->tp, balloc);
diff --git a/test/Makefile.in b/test/Makefile.in
index d32fb31..4af81ab 100644
--- a/test/Makefile.in
+++ b/test/Makefile.in
@@ -52,6 +52,7 @@
proc_child@EXEEXT@ \
readchild@EXEEXT@ \
sockchild@EXEEXT@ \
+ memcachedmock@EXEEXT@ \
testshmproducer@EXEEXT@ \
testshmconsumer@EXEEXT@ \
tryread@EXEEXT@ \
@@ -156,6 +157,10 @@
sockchild@EXEEXT@: $(OBJECTS_sockchild)
$(LINK_PROG) $(OBJECTS_sockchild) $(ALL_LIBS)
+OBJECTS_memcachedmock = memcachedmock.lo $(LOCAL_LIBS)
+memcachedmock@EXEEXT@: $(OBJECTS_memcachedmock)
+ $(LINK_PROG) $(OBJECTS_memcachedmock) $(ALL_LIBS)
+
OBJECTS_testshmconsumer = testshmconsumer.lo $(LOCAL_LIBS)
testshmconsumer@EXEEXT@: $(OBJECTS_testshmconsumer) $(LOCAL_LIBS)
$(LINK_PROG) $(OBJECTS_testshmconsumer) $(ALL_LIBS)
diff --git a/test/Makefile.win b/test/Makefile.win
index 21344ea..ff96bee 100644
--- a/test/Makefile.win
+++ b/test/Makefile.win
@@ -73,6 +73,7 @@
$(OUTDIR)\proc_child.exe \
$(OUTDIR)\tryread.exe \
$(OUTDIR)\sockchild.exe \
+ $(OUTDIR)\memcachedmock.exe \
$(OUTDIR)\testshmproducer.exe \
$(OUTDIR)\testshmconsumer.exe \
$(OUTDIR)\globalmutexchild.exe
@@ -274,6 +275,11 @@
@if exist "$@.manifest" \
mt.exe -manifest "$@.manifest" -outputresource:$@;1
+$(OUTDIR)\memcachedmock.exe: $(INTDIR)\memcachedmock.obj $(LOCAL_LIB)
+ $(LD) $(LDFLAGS) /out:"$@" $** $(LD_LIBS)
+ @if exist "$@.manifest" \
+ mt.exe -manifest "$@.manifest" -outputresource:$@;1
+
$(OUTDIR)\testshmconsumer.exe: $(INTDIR)\testshmconsumer.obj $(LOCAL_LIB)
$(LD) $(LDFLAGS) /out:"$@" $** $(LD_LIBS)
@if exist "$@.manifest" \
diff --git a/test/NWGNUmakefile b/test/NWGNUmakefile
index 350e665..a26d2c8 100644
--- a/test/NWGNUmakefile
+++ b/test/NWGNUmakefile
@@ -169,6 +169,7 @@
$(OBJDIR)/proc_child.nlm \
$(OBJDIR)/readchild.nlm \
$(OBJDIR)/sockchild.nlm \
+ $(OBJDIR)/memcachedmock.nlm \
$(OBJDIR)/sockperf.nlm \
$(OBJDIR)/testatmc.nlm \
$(OBJDIR)/tryread.nlm \
diff --git a/test/NWGNUmemcachedmock b/test/NWGNUmemcachedmock
new file mode 100644
index 0000000..db35f34
--- /dev/null
+++ b/test/NWGNUmemcachedmock
@@ -0,0 +1,252 @@
+#
+# Make sure all needed macro's are defined
+#
+
+#
+# Get the 'head' of the build environment if necessary. This includes default
+# targets and paths to tools
+#
+
+ifndef EnvironmentDefined
+include $(APR_WORK)/build/NWGNUhead.inc
+endif
+
+#
+# These directories will be at the beginning of the include list, followed by
+# INCDIRS
+#
+XINCDIRS += \
+ $(APR)/include \
+ $(APR)/include/arch/netware \
+ $(EOLIST)
+
+#
+# These flags will come after CFLAGS
+#
+XCFLAGS += \
+ $(EOLIST)
+
+#
+# These defines will come after DEFINES
+#
+XDEFINES += \
+ $(EOLIST)
+
+#
+# These flags will be added to the link.opt file
+#
+XLFLAGS += \
+ $(EOLIST)
+
+#
+# These values will be appended to the correct variables based on the value of
+# RELEASE
+#
+ifeq "$(RELEASE)" "debug"
+XINCDIRS += \
+ $(EOLIST)
+
+XCFLAGS += \
+ $(EOLIST)
+
+XDEFINES += \
+ $(EOLIST)
+
+XLFLAGS += \
+ $(EOLIST)
+endif
+
+ifeq "$(RELEASE)" "noopt"
+XINCDIRS += \
+ $(EOLIST)
+
+XCFLAGS += \
+ $(EOLIST)
+
+XDEFINES += \
+ $(EOLIST)
+
+XLFLAGS += \
+ $(EOLIST)
+endif
+
+ifeq "$(RELEASE)" "release"
+XINCDIRS += \
+ $(EOLIST)
+
+XCFLAGS += \
+ $(EOLIST)
+
+XDEFINES += \
+ $(EOLIST)
+
+XLFLAGS += \
+ $(EOLIST)
+endif
+
+#
+# These are used by the link target if an NLM is being generated
+# This is used by the link 'name' directive to name the nlm. If left blank
+# TARGET_nlm (see below) will be used.
+#
+NLM_NAME = memcachedmock
+
+#
+# This is used by the link '-desc ' directive.
+# If left blank, NLM_NAME will be used.
+#
+NLM_DESCRIPTION = socket NLM to test sockets
+
+#
+# This is used by the '-threadname' directive. If left blank,
+# NLM_NAME Thread will be used.
+#
+NLM_THREAD_NAME = $(NLM_NAME)
+
+#
+# This is used by the '-screenname' directive. If left blank,
+# 'Apache for NetWare' Thread will be used.
+#
+NLM_SCREEN_NAME = DEFAULT
+
+#
+# If this is specified, it will override VERSION value in
+# $(APR_WORK)/build/NWGNUenvironment.inc
+#
+NLM_VERSION =
+
+#
+# If this is specified, it will override the default of 64K
+#
+NLM_STACK_SIZE =
+
+#
+# If this is specified it will be used by the link '-entry' directive
+#
+NLM_ENTRY_SYM =
+
+#
+# If this is specified it will be used by the link '-exit' directive
+#
+NLM_EXIT_SYM =
+
+#
+# If this is specified it will be used by the link '-check' directive
+#
+NLM_CHECK_SYM =
+
+#
+# If this is specified it will be used by the link '-flags' directive
+#
+NLM_FLAGS = AUTOUNLOAD, PSEUDOPREEMPTION, MULTIPLE
+
+#
+# If this is specified it will be linked in with the XDCData option in the def
+# file instead of the default of $(APR)/misc/netware/apache.xdc. XDCData can
+# be disabled by setting APACHE_UNIPROC in the environment
+#
+XDCDATA =
+
+#
+# Declare all target files (you must add your files here)
+#
+
+#
+# If there is an NLM target, put it here
+#
+TARGET_nlm = \
+ $(OBJDIR)/$(NLM_NAME).nlm \
+ $(EOLIST)
+
+#
+# If there is an LIB target, put it here
+#
+TARGET_lib = \
+ $(EOLIST)
+
+#
+# These are the OBJ files needed to create the NLM target above.
+# Paths must all use the '/' character
+#
+FILES_nlm_objs = \
+ $(OBJDIR)/$(NLM_NAME).o \
+ $(EOLIST)
+
+#
+# These are the LIB files needed to create the NLM target above.
+# These will be added as a library command in the link.opt file.
+#
+FILES_nlm_libs = \
+ $(PRELUDE) \
+ $(EOLIST)
+
+#
+# These are the modules that the above NLM target depends on to load.
+# These will be added as a module command in the link.opt file.
+#
+FILES_nlm_modules = \
+ aprlib \
+ libc \
+ $(EOLIST)
+
+#
+# If the nlm has a msg file, put it's path here
+#
+FILE_nlm_msg =
+
+#
+# If the nlm has a hlp file put it's path here
+#
+FILE_nlm_hlp =
+
+#
+# If this is specified, it will override the default copyright.
+#
+FILE_nlm_copyright =
+
+#
+# Any additional imports go here
+#
+FILES_nlm_Ximports = \
+ @$(APR)/aprlib.imp \
+ @$(NOVI)/libc.imp \
+ $(EOLIST)
+
+#
+# Any symbols exported to here
+#
+FILES_nlm_exports = \
+ $(EOLIST)
+
+#
+# These are the OBJ files needed to create the LIB target above.
+# Paths must all use the '/' character
+#
+FILES_lib_objs = \
+ $(EOLIST)
+
+#
+# implement targets and dependancies (leave this section alone)
+#
+
+libs :: $(OBJDIR) $(TARGET_lib)
+
+nlms :: libs $(TARGET_nlm)
+
+#
+# Updated this target to create necessary directories and copy files to the
+# correct place. (See $(APR_WORK)/build/NWGNUhead.inc for examples)
+#
+install :: nlms FORCE
+
+#
+# Any specialized rules here
+#
+
+#
+# Include the 'tail' makefile that has targets that depend on variables defined
+# in this makefile
+#
+
+include $(APRBUILD)/NWGNUtail.inc
+
diff --git a/test/memcachedmock.c b/test/memcachedmock.c
new file mode 100644
index 0000000..b6c2e6f
--- /dev/null
+++ b/test/memcachedmock.c
@@ -0,0 +1,84 @@
+/* Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include <stdlib.h>
+#include "apr_network_io.h"
+#include "apr_pools.h"
+#include "apr_pools.h"
+#include "testmemcache.h"
+
+#define MOCK_REPLY "VERSION 1.5.22\r\n"
+
+int main(int argc, char *argv[])
+{
+ apr_pool_t *p;
+ apr_sockaddr_t *sa;
+ apr_socket_t *server;
+ apr_socket_t *server_connection;
+ apr_status_t rv;
+ apr_size_t length;
+ int i;
+
+ apr_initialize();
+ atexit(apr_terminate);
+ apr_pool_create(&p, NULL);
+
+ apr_sockaddr_info_get(&sa, MOCK_HOST, APR_UNSPEC, MOCK_PORT, 0, p);
+
+ apr_socket_create(&server, sa->family, SOCK_STREAM, 0, p);
+
+ apr_socket_opt_set(server, APR_SO_REUSEADDR, 1);
+
+ apr_socket_timeout_set(server, 0);
+
+ apr_socket_bind(server, sa);
+
+ apr_socket_listen(server, 5);
+
+ /* Do spin instead of a proper poll for sake of simplicity */
+ for (i = 0; i < 4; i++) {
+
+ rv = apr_socket_accept(&server_connection, server, p);
+ if (rv == APR_SUCCESS) {
+ break;
+ }
+
+ apr_sleep(apr_time_from_sec(1));
+ }
+
+ length = strlen(MOCK_REPLY);
+ apr_socket_send(server_connection, MOCK_REPLY, &length);
+
+ apr_socket_close(server_connection);
+
+ /* Do spin instead of a proper poll for sake of simplicity */
+ for (i = 0; i < 4; i++) {
+
+ rv = apr_socket_accept(&server_connection, server, p);
+ if (rv == APR_SUCCESS) {
+ break;
+ }
+
+ apr_sleep(apr_time_from_sec(1));
+ }
+
+ length = strlen(MOCK_REPLY);
+ apr_socket_send(server_connection, MOCK_REPLY, &length);
+
+ apr_socket_close(server_connection);
+
+ exit(0);
+}
diff --git a/test/testmemcache.c b/test/testmemcache.c
index 33e0e7c..20fad58 100644
--- a/test/testmemcache.c
+++ b/test/testmemcache.c
@@ -23,6 +23,8 @@
#include "apr_memcache.h"
#include "apr_network_io.h"
#include "apr_thread_proc.h"
+#include "apr_signal.h"
+#include "testmemcache.h"
#if APR_HAVE_STDLIB_H
#include <stdlib.h> /* for exit() */
@@ -662,6 +664,72 @@
}
}
+static void test_connection_validation(abts_case *tc, void *data)
+{
+ apr_status_t rv;
+ apr_memcache_t *memcache;
+ apr_memcache_server_t *memserver;
+ char *result;
+ apr_procattr_t *procattr;
+ apr_proc_t proc;
+ const char *args[2];
+ int exitcode;
+ apr_exit_why_e why;
+#ifdef SIGPIPE
+ /*
+ * If SIGPIPE is present ignore it as we will write to a closed socket.
+ * Otherwise we would be terminated by the default handler for SIGPIPE.
+ */
+ apr_sigfunc_t *old_action;
+
+ old_action = apr_signal(SIGPIPE, SIG_IGN);
+#endif
+
+ rv = apr_procattr_create(&procattr, p);
+ ABTS_ASSERT(tc, "Couldn't create procattr", rv == APR_SUCCESS);
+
+ rv = apr_procattr_io_set(procattr, APR_NO_PIPE, APR_NO_PIPE,
+ APR_NO_PIPE);
+ ABTS_ASSERT(tc, "Couldn't set io in procattr", rv == APR_SUCCESS);
+
+ rv = apr_procattr_error_check_set(procattr, 1);
+ ABTS_ASSERT(tc, "Couldn't set error check in procattr", rv == APR_SUCCESS);
+
+ rv = apr_procattr_cmdtype_set(procattr, APR_PROGRAM_ENV);
+ ABTS_ASSERT(tc, "Couldn't set copy environment", rv == APR_SUCCESS);
+
+ args[0] = "memcachedmock" EXTENSION;
+ args[1] = NULL;
+ rv = apr_proc_create(&proc, TESTBINPATH "memcachedmock" EXTENSION, args, NULL,
+ procattr, p);
+ ABTS_ASSERT(tc, "Couldn't launch program", rv == APR_SUCCESS);
+
+ /* Wait for the mock memcached to start */
+ apr_sleep(apr_time_from_sec(2));
+
+ rv = apr_memcache_create(p, 1, 0, &memcache);
+ ABTS_ASSERT(tc, "memcache create failed", rv == APR_SUCCESS);
+
+ rv = apr_memcache_server_create(p, MOCK_HOST, MOCK_PORT, 0, 1, 1, 60000, &memserver);
+ ABTS_ASSERT(tc, "server create failed", rv == APR_SUCCESS);
+
+ rv = apr_memcache_add_server(memcache, memserver);
+ ABTS_ASSERT(tc, "server add failed", rv == APR_SUCCESS);
+
+ rv = apr_memcache_version(memserver, p, &result);
+ ABTS_ASSERT(tc, "Couldn't get initial version", rv == APR_SUCCESS);
+
+ rv = apr_memcache_version(memserver, p, &result);
+ ABTS_ASSERT(tc, "Couldn't get version after connection shutdown", rv == APR_SUCCESS);
+
+#ifdef SIGPIPE
+ /* Restore old SIGPIPE handler */
+ apr_signal(SIGPIPE, old_action);
+#endif
+
+ apr_proc_wait(&proc, &exitcode, &why, APR_WAIT);
+}
+
abts_suite *testmemcache(abts_suite * suite)
{
suite = ADD_SUITE(suite);
@@ -672,6 +740,7 @@
abts_run_test(suite, test_memcache_multiget, NULL);
abts_run_test(suite, test_memcache_addreplace, NULL);
abts_run_test(suite, test_memcache_incrdecr, NULL);
+ abts_run_test(suite, test_connection_validation, NULL);
return suite;
}
diff --git a/test/testmemcache.h b/test/testmemcache.h
new file mode 100644
index 0000000..e7f7066
--- /dev/null
+++ b/test/testmemcache.h
@@ -0,0 +1,24 @@
+/* Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#ifndef TESTMEMCACHE_H
+#define TESTMEMCACHE_H
+
+#define MOCK_HOST "localhost"
+#define MOCK_PORT 11231
+
+#endif
+
