[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Handling assertions and malfunctions in mod_dav_svn

From: Evgeny Kotkov <evgeny.kotkov_at_visualsvn.com>
Date: Tue, 17 Feb 2015 04:21:57 +0300

Subversion's mod_dav_svn module currently uses a default malfunction handler,
svn_error_abort_on_malfunction(). This means that SVN_ERR_MALFUNCTION() or
failed SVN_ERR_ASSERT() statements output the malfunction details to STDERR
and abort() the current process. Doing so within an Apache module — as opposed
to a command-line program like svn — has two potential problems:

A) Logging

   A default malfunction handler calls svn_handle_error2(), which prints the
   malfunction details to STDERR. Apache replaces STDERR with what ErrorLog
   directive [1] points to, and the information about a malfunction goes right
   to it. However, a couple of caveats apply.

   First of all, doing so bypasses existing error log hooks installed via
   ap_hook_error_log(). I think that there might be existing installations
   that happen to use custom error log hooks as an addition to the main error
   log or even as a complete replacement for it (think ErrorLog /dev/null, but
   with a custom logging module doing all the work). In the latter case, the
   information about the malfunction would be completely lost.

   Secondly, the error message contains an unnecessary "svn:" prefix, and this
   is inconsistent with other Subversion errors that can appear in the Apache
   error log.

B) Taking down other threads

   Calling abort() means we take down the entire process, with a respect to the
   currently chosen MPM. We cannot assume that this process does not contain
   other, non-mod_dav_svn threads and we might want to be conservative about
   abort()'s here, as just killing everything could do more harm than good.

Here are several possible options:

1) Install a mod_dav_svn malfunction handler that calls ap_log_assert();
   ap_log_assert() also terminates the current process, so this means we
   would keep the existing abortive behavior.

   Pros: Solves A)

   Cons: Doesn't really do anything about B)

2) Install a non-abortive mod_dav_svn malfunction handler, which would log the
   malfunction details using ap_log_error() and return SVN_ERR_ASSERTION_FAIL
   when CAN_RETURN = TRUE. This option is similiar to the current behavior of
   a non-default malfunction handler, svn_error_raise_on_malfunction().

   (See svn_error_malfunction_handler_t for details about CAN_RETURN argument)

   Pros:
   Doing so would address both A) and a huge subset of B). Only a subset,
   because there still are situations with CAN_RETURN = FALSE when we really
   cannot do something graceful in a malfunction handler rather than abort().

   Cons:
   Our codebase currently includes ~ 300 assertions, which might eventually get
   triggered by mod_dav_svn. We might encounter situations with the callers
   ignoring an SVN_ERR_ASSERTION_FAIL. Say, they could svn_error_clear()
   it. Or turn it into dav_error *, and then mod_dav would ignore this error
   for some reasons and continue to call us as if nothing happened. This could
   result in us running in an undefined state — i.e., where abort() would have
   probably been a better choice.

3) Add a SVNAbortOnMalfunction directive (default is On). Install different
   malfunction handlers based on the value of this flag. Describe what's the
   purpose of this directive and announce an intention to change the default to
   Off in the next minor release. Let it soak, and change the default.

   Pros:
   Same as 2), but an administrator can make a better choice based on his/her
   Apache configuration.

   Cons:
   Requires time. Does not address B) with the default value.

Thoughts?

[1] http://httpd.apache.org/docs/2.4/mod/core.html#errorlog

Regards,
Evgeny Kotkov
Received on 2015-02-17 02:23:31 CET

This is an archived mail posted to the Subversion Dev mailing list.