You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by st...@apache.org on 2013/09/15 22:39:28 UTC
svn commit: r1523502 - in /subversion/trunk/subversion/svnserve: serve.c
server.h svnserve.c
Author: stefan2
Date: Sun Sep 15 20:39:27 2013
New Revision: 1523502
URL: http://svn.apache.org/r1523502
Log:
Fix svnserve logging in multi-threaded servers and make them log all errors.
We simply use a mutex alongside the log file to serialize all access to it.
That also allows us to use the log file in our worker thread funtion.
* subversion/svnserve/server.h
(server_baton_t,
serve_params_t): add mutexes next to the log file objects
(log_error): extend signature
* subversion/svnserve/serve.c
(log_error_internal): factored out from log_error
(log_error): add serialization support
(log_server_error,
serve): update callers
* subversion/svnserve/svnserve.c
(serve_socket): also log errors
(main): update caller
Modified:
subversion/trunk/subversion/svnserve/serve.c
subversion/trunk/subversion/svnserve/server.h
subversion/trunk/subversion/svnserve/svnserve.c
Modified: subversion/trunk/subversion/svnserve/serve.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svnserve/serve.c?rev=1523502&r1=1523501&r2=1523502&view=diff
==============================================================================
--- subversion/trunk/subversion/svnserve/serve.c (original)
+++ subversion/trunk/subversion/svnserve/serve.c Sun Sep 15 20:39:27 2013
@@ -112,21 +112,16 @@ log_write(apr_file_t *log_file, const ch
return svn_io_file_write(log_file, errstr, &len, pool);
}
-void
-log_error(svn_error_t *err, apr_file_t *log_file, const char *remote_host,
- const char *user, const char *repos, apr_pool_t *pool)
+static void
+log_error_internal(svn_error_t *err, apr_file_t *log_file,
+ const char *remote_host, const char *user,
+ const char *repos, apr_pool_t *pool)
{
const char *timestr, *continuation;
char errbuf[256];
/* 8192 from MAX_STRING_LEN in from httpd-2.2.4/include/httpd.h */
char errstr[8192];
- if (err == SVN_NO_ERROR)
- return;
-
- if (log_file == NULL)
- return;
-
timestr = svn_time_to_cstring(apr_time_now(), pool);
remote_host = (remote_host ? remote_host : "-");
user = (user ? user : "-");
@@ -160,13 +155,41 @@ log_error(svn_error_t *err, apr_file_t *
}
}
+
+void
+log_error(svn_error_t *err, apr_file_t *log_file,
+ svn_mutex__t *log_file_mutex, const char *remote_host,
+ const char *user, const char *repos, apr_pool_t *pool)
+{
+ if (err == SVN_NO_ERROR)
+ return;
+
+ if (log_file == NULL)
+ return;
+
+ if (log_file_mutex)
+ {
+ /* if the mutex functions fail, things look pretty grim.
+ *
+ * Our best hope is to try to get some info into the log before
+ * everything breaks apart. Therefore, ignore any mutex errors.
+ */
+ svn_error_clear(svn_mutex__lock(log_file_mutex));
+ log_error_internal(err, log_file, remote_host, user, repos, pool);
+ svn_error_clear(svn_mutex__unlock(log_file_mutex, SVN_NO_ERROR));
+ }
+ else
+ log_error_internal(err, log_file, remote_host, user, repos, pool);
+}
+
/* Call log_error with log_file, remote_host, user, and repos
arguments from SERVER and CONN. */
static void
log_server_error(svn_error_t *err, server_baton_t *server,
svn_ra_svn_conn_t *conn, apr_pool_t *pool)
{
- log_error(err, server->log_file, svn_ra_svn_conn_remote_host(conn),
+ log_error(err, server->log_file, server->log_file_mutex,
+ svn_ra_svn_conn_remote_host(conn),
server->user, server->repos_name, pool);
}
@@ -3531,6 +3554,7 @@ svn_error_t *serve(svn_ra_svn_conn_t *co
b.authzdb = NULL;
b.realm = NULL;
b.log_file = params->log_file;
+ b.log_file_mutex = params->log_file_mutex;
b.pool = pool;
b.use_sasl = FALSE;
b.vhost = params->vhost;
@@ -3635,8 +3659,8 @@ svn_error_t *serve(svn_ra_svn_conn_t *co
}
if (err)
{
- log_error(err, b.log_file, svn_ra_svn_conn_remote_host(conn),
- b.user, NULL, pool);
+ log_error(err, b.log_file, b.log_file_mutex,
+ svn_ra_svn_conn_remote_host(conn), b.user, NULL, pool);
io_err = svn_ra_svn__write_cmd_failure(conn, pool, err);
svn_error_clear(err);
SVN_ERR(io_err);
Modified: subversion/trunk/subversion/svnserve/server.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svnserve/server.h?rev=1523502&r1=1523501&r2=1523502&view=diff
==============================================================================
--- subversion/trunk/subversion/svnserve/server.h (original)
+++ subversion/trunk/subversion/svnserve/server.h Sun Sep 15 20:39:27 2013
@@ -36,6 +36,8 @@ extern "C" {
#include "svn_repos.h"
#include "svn_ra_svn.h"
+#include "private/svn_mutex.h"
+
enum username_case_type { CASE_FORCE_UPPER, CASE_FORCE_LOWER, CASE_ASIS };
typedef struct server_baton_t {
@@ -60,6 +62,8 @@ typedef struct server_baton_t {
svn_boolean_t use_sasl; /* Use Cyrus SASL for authentication;
always false if SVN_HAVE_SASL not defined */
apr_file_t *log_file; /* Log filehandle. */
+ svn_mutex__t *log_file_mutex; /* Serializes access to log_file.
+ May be NULL even if log_file is not. */
svn_boolean_t vhost; /* Use virtual-host-based path to repo. */
apr_pool_t *pool;
} server_baton_t;
@@ -100,6 +104,9 @@ typedef struct serve_params_t {
/* A filehandle open for writing logs to; possibly NULL. */
apr_file_t *log_file;
+ /* Mutex to serialize access to log_file; possibly NULL. */
+ svn_mutex__t *log_file_mutex;
+
/* Username case normalization style. */
enum username_case_type username_case;
@@ -162,11 +169,14 @@ svn_error_t *cyrus_auth_request(svn_ra_s
apr_size_t escape_errorlog_item(char *dest, const char *source,
apr_size_t buflen);
-/* Log ERR to LOG_FILE if LOG_FILE is not NULL. Include REMOTE_HOST,
- USER, and REPOS in the log if they are not NULL. Allocate temporary
- char buffers in POOL (which caller can then clear or dispose of). */
+/* Log ERR to LOG_FILE if LOG_FILE is not NULL. In multi-threaded
+ servers, you should also provide a LOG_FILE_MUTEX object to serialize
+ access to the log file. Include REMOTE_HOST, USER, and REPOS in the
+ log if they are not NULL. Allocate temporary char buffers in POOL
+ (which caller can then clear or dispose of). */
void
-log_error(svn_error_t *err, apr_file_t *log_file, const char *remote_host,
+log_error(svn_error_t *err, apr_file_t *log_file,
+ svn_mutex__t *log_file_mutex, const char *remote_host,
const char *user, const char *repos, apr_pool_t *pool);
#ifdef __cplusplus
Modified: subversion/trunk/subversion/svnserve/svnserve.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svnserve/svnserve.c?rev=1523502&r1=1523501&r2=1523502&view=diff
==============================================================================
--- subversion/trunk/subversion/svnserve/svnserve.c (original)
+++ subversion/trunk/subversion/svnserve/svnserve.c Sun Sep 15 20:39:27 2013
@@ -32,6 +32,7 @@
#include <apr_thread_proc.h>
#include <apr_thread_pool.h>
#include <apr_portable.h>
+#include <apr_poll.h>
#include <locale.h>
@@ -549,6 +550,7 @@ serve_socket(apr_socket_t *usock,
{
apr_status_t status;
svn_ra_svn_conn_t *conn;
+ svn_error_t *err;
/* Enable TCP keep-alives on the socket so we time out when
* the connection breaks due to network-layer problems.
@@ -571,10 +573,13 @@ serve_socket(apr_socket_t *usock,
params->error_check_interval,
pool);
- /* process the actual request */
- SVN_ERR(serve(conn, params, pool));
+ /* process the actual request and log errors */
+ err = serve(conn, params, pool);
+ if (err)
+ log_error(err, params->log_file, params->log_file_mutex,
+ svn_ra_svn_conn_remote_host(conn), NULL, NULL, pool);
- return SVN_NO_ERROR;
+ return svn_error_trace(err);
}
/* "Arguments" passed from the main thread to the connection thread */
@@ -650,7 +655,6 @@ int main(int argc, const char *argv[])
serve_params_t params;
const char *arg;
apr_status_t status;
- svn_ra_svn_conn_t *conn;
apr_proc_t proc;
#if APR_HAS_THREADS
apr_thread_pool_t *threads;
@@ -708,6 +712,7 @@ int main(int argc, const char *argv[])
params.cfg = NULL;
params.compression_level = SVN_DELTA_COMPRESSION_LEVEL_DEFAULT;
params.log_file = NULL;
+ params.log_file_mutex = NULL;
params.vhost = FALSE;
params.username_case = CASE_ASIS;
params.memory_cache_size = (apr_uint64_t)-1;
@@ -963,9 +968,12 @@ int main(int argc, const char *argv[])
}
if (log_filename)
- SVN_INT_ERR(svn_io_file_open(¶ms.log_file, log_filename,
- APR_WRITE | APR_CREATE | APR_APPEND,
- APR_OS_DEFAULT, pool));
+ {
+ SVN_INT_ERR(svn_io_file_open(¶ms.log_file, log_filename,
+ APR_WRITE | APR_CREATE | APR_APPEND,
+ APR_OS_DEFAULT, pool));
+ SVN_INT_ERR(svn_mutex__init(¶ms.log_file_mutex, TRUE, pool));
+ }
if (params.tunnel_user && run_mode != run_mode_tunnel)
{
@@ -978,6 +986,8 @@ int main(int argc, const char *argv[])
if (run_mode == run_mode_inetd || run_mode == run_mode_tunnel)
{
+ svn_ra_svn_conn_t *conn;
+
params.tunnel = (run_mode == run_mode_tunnel);
apr_pool_cleanup_register(pool, pool, apr_pool_cleanup_null,
redirect_stdout);
@@ -1269,12 +1279,7 @@ int main(int argc, const char *argv[])
if (status == APR_INCHILD)
{
apr_socket_close(sock);
- err = serve_socket(usock, ¶ms, connection_pool);
- log_error(err, params.log_file,
- svn_ra_svn_conn_remote_host(conn),
- NULL, NULL, /* user, repos */
- connection_pool);
- svn_error_clear(err);
+ svn_error_clear(serve_socket(usock, ¶ms, connection_pool));
apr_socket_close(usock);
exit(0);
}
@@ -1285,9 +1290,8 @@ int main(int argc, const char *argv[])
else
{
err = svn_error_wrap_apr(status, "apr_proc_fork");
- log_error(err, params.log_file,
- svn_ra_svn_conn_remote_host(conn),
- NULL, NULL, /* user, repos */
+ log_error(err, params.log_file, params.log_file_mutex,
+ NULL, NULL, NULL, /* ip, user, repos */
connection_pool);
svn_error_clear(err);
apr_socket_close(usock);