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