Kouhei Sutou wrote:
> I got the same backtrace and my patch fixed the problem on
> my environment. Could you try it if you didn't try it yet?
>
> (I don't know that the patch can be still applied into
> trunk. sorry.)
>
> Index: subversion/libsvn_delta/text_delta.c
> ===================================================================
> --- svn/subversion/libsvn_delta/text_delta.c (revision 35611)
> +++ svn/subversion/libsvn_delta/text_delta.c (working copy)
> @@ -788,8 +788,11 @@
> apr_pool_t *pool)
> {
> svn_txdelta_stream_t *txstream;
> + apr_pool_t *sub_pool;
> svn_error_t *err;
>
> + sub_pool = svn_pool_create(pool);
> +
> /* ### this is a hack. we should simply read from the stream, construct
> ### some windows, and pass those to the handler. there isn't any reason
> ### to crank up a full "diff" algorithm just to copy a stream.
> @@ -798,8 +801,8 @@
>
> /* Create a delta stream which converts an *empty* bytestream into the
> target bytestream. */
> - svn_txdelta(&txstream, svn_stream_empty(pool), stream, pool);
> - err = svn_txdelta_send_txstream(txstream, handler, handler_baton, pool);
> + svn_txdelta(&txstream, svn_stream_empty(sub_pool), stream, sub_pool);
> + err = svn_txdelta_send_txstream(txstream, handler, handler_baton, sub_pool);
>
> if (digest && (! err))
> {
> @@ -809,6 +812,8 @@
> memcpy(digest, result_md5, APR_MD5_DIGESTSIZE);
> }
>
> + svn_pool_destroy(sub_pool);
> +
> return err;
> }
Do you (or does anyone else) know *why* the patch works, though? I ask
because I'm concerned that we're looking at a memory-related issue that
shows up only for some folks because of the way the bits land on their
machines, and that isn't *fixed* by your patch so much as masked simply
because the code path changes a little and a different pool is used.
Your patch create a subpool of pool and uses it everywhere pool was used,
then destroys the subpool. The lifetime of your subpool is certainly
shorter than the lifetime of pool except when an error happens, so I
wouldn't expect this to actually remedy a pool lifetime problem. So, if we
can't explain exactly why your patch helps and what problem it solves, my
vote is -1 on applying it. We don't want to make the tests stop failing; we
want to make Subversion work correctly (and we'll happily accept passing
tests as a benefit of that change).
--
C. Michael Pilato <cmpilato_at_collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1129263
Received on 2009-02-09 15:29:00 CET