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

Re: svn commit: r995478 [1/2] - in /subversion/branches/performance: ./ subversion/bindings/javahl/native/ subversion/bindings/javahl/src/org/apache/subversion/javahl/ subversion/bindings/javahl/src/org/tigris/subversion/javahl/ subversion/bindings/j

From: Hyrum Wright <hwright_at_apache.org>
Date: Thu, 9 Sep 2010 18:45:11 -0500

On Thu, Sep 9, 2010 at 4:34 PM, Stefan Fuhrmann
<stefanfuhrmann_at_alice-dsl.de> wrote:
> Hyrum Wright wrote:
>>
>> On Thu, Sep 9, 2010 at 10:59 AM, †<hwright_at_apache.org> wrote:
>>
>>>
>>> Author: hwright
>>> Date: Thu Sep †9 15:59:00 2010
>>> New Revision: 995478
>>>
>>> URL: http://svn.apache.org/viewvc?rev=995478&view=rev
>>> Log:
>>> On the performance branch:
>>> Bring up-to-date with trunk.
>>>
>>
>> ...
>>
>>>
>>> Modified: subversion/branches/performance/subversion/libsvn_subr/io.c
>>> URL:
>>> http://svn.apache.org/viewvc/subversion/branches/performance/subversion/libsvn_subr/io.c?rev=995478&r1=995477&r2=995478&view=diff
>>>
>>> ==============================================================================
>>> --- subversion/branches/performance/subversion/libsvn_subr/io.c
>>> (original)
>>> +++ subversion/branches/performance/subversion/libsvn_subr/io.c Thu Sep
>>> †9 15:59:00 2010
>>> @@ -1076,14 +1076,19 @@ svn_error_t *svn_io_file_create(const ch
>>> †{
>>> †apr_file_t *f;
>>> †apr_size_t written;
>>> + †svn_error_t *err;
>>>
>>> †SVN_ERR(svn_io_file_open(&f, file,
>>> † † † † † † † † † † † † † (APR_WRITE | APR_CREATE | APR_EXCL),
>>> † † † † † † † † † † † † † APR_OS_DEFAULT,
>>> † † † † † † † † † † † † † pool));
>>> - †SVN_ERR(svn_io_file_write_full(f, contents, strlen(contents),
>>> - † † † † † † † † † † † † † † † † &written, pool));
>>> - †return svn_io_file_close(f, pool);
>>> + †err= svn_io_file_write_full(f, contents, strlen(contents),
>>> + † † † † † † † † † † † † † † †&written, pool);
>>> +
>>> +
>>> + †return svn_error_return(
>>> + † † † † † † † † † † † †svn_error_compose_create(err,
>>> + † † † † † † † † † † † † † † † † † † † † † † † † svn_io_file_close(f,
>>> pool)));
>>> †}
>>>
>>> †svn_error_t *svn_io_dir_file_copy(const char *src_path,
>>> @@ -2952,12 +2957,19 @@ svn_io_write_unique(const char **tmp_pat
>>> † † † † † † † † † †apr_pool_t *pool)
>>> †{
>>> †apr_file_t *new_file;
>>> + †svn_error_t *err;
>>>
>>> †SVN_ERR(svn_io_open_unique_file3(&new_file, tmp_path, dirpath,
>>> † † † † † † † † † † † † † † † † † delete_when, pool, pool));
>>> - †SVN_ERR(svn_io_file_write_full(new_file, buf, nbytes, NULL, pool));
>>> - †SVN_ERR(svn_io_file_flush_to_disk(new_file, pool));
>>> - †return svn_io_file_close(new_file, pool);
>>> +
>>> + †err = svn_io_file_write_full(new_file, buf, nbytes, NULL, pool);
>>> +
>>> + †if (!err)
>>> + † †err = svn_io_file_flush_to_disk(new_file, pool);
>>> +
>>> + †return svn_error_return(
>>> + † † † † † † † † †svn_error_compose_create(err,
>>> + † † † † † † † † † † † † † † † † † † † † † svn_io_file_close(new_file,
>>> pool)));
>>> †}
>>>
>>>
>>> @@ -3521,15 +3533,17 @@ svn_io_read_version_file(int *version,
>>> †apr_file_t *format_file;
>>> †char buf[80];
>>> †apr_size_t len;
>>> + †svn_error_t *err;
>>>
>>> †/* Read a chunk of data from PATH */
>>> †SVN_ERR(svn_io_file_open(&format_file, path, APR_READ,
>>> † † † † † † † † † † † † † APR_OS_DEFAULT, pool));
>>> †len = sizeof(buf);
>>> - †SVN_ERR(svn_io_file_read(format_file, buf, &len, pool));
>>> + †err = svn_io_file_read(format_file, buf, &len, pool);
>>>
>>> †/* Close the file. */
>>> - †SVN_ERR(svn_io_file_close(format_file, pool));
>>> + †SVN_ERR(svn_error_compose_create(err,
>>> + † † † † † † † † † † † † † † † † † svn_io_file_close(format_file,
>>> pool)));
>>>
>>> †/* If there was no data in PATH, return an error. */
>>> †if (len == 0)
>>> @@ -3556,7 +3570,7 @@ svn_io_read_version_file(int *version,
>>> †}
>>>
>>> †/* Convert to integer. */
>>> - †*version = atoi(buf);
>>> + †SVN_ERR(svn_cstring_atoi(version, buf));
>>>
>>> †return SVN_NO_ERROR;
>>> †}
>>> @@ -3570,40 +3584,65 @@ contents_identical_p(svn_boolean_t *iden
>>> † † † † † † † † † † const char *file2,
>>> † † † † † † † † † † apr_pool_t *pool)
>>> †{
>>> + †svn_error_t *err;
>>> †apr_size_t bytes_read1, bytes_read2;
>>> †char *buf1 = apr_palloc(pool, SVN__STREAM_CHUNK_SIZE);
>>> †char *buf2 = apr_palloc(pool, SVN__STREAM_CHUNK_SIZE);
>>> †apr_file_t *file1_h = NULL;
>>> †apr_file_t *file2_h = NULL;
>>> + †svn_boolean_t done1 = FALSE;
>>> + †svn_boolean_t done2 = FALSE;
>>>
>>> †SVN_ERR(svn_io_file_open(&file1_h, file1, APR_READ, APR_OS_DEFAULT,
>>> † † † † † † † † † † † † † pool));
>>> - †SVN_ERR(svn_io_file_open(&file2_h, file2, APR_READ, APR_OS_DEFAULT,
>>> - † † † † † † † † † † † † † pool));
>>> +
>>> + †err = svn_io_file_open(&file2_h, file2, APR_READ, APR_OS_DEFAULT,
>>> + † † † † † † † † † † † † pool);
>>> +
>>> + †if (err)
>>> + † †return svn_error_return(
>>> + † † † † † † † svn_error_compose_create(err,
>>> + † † † † † † † † † † † † † † † † † † † †svn_io_file_close(file1_h,
>>> pool)));
>>>
>>> †*identical_p = TRUE; †/* assume TRUE, until disproved below */
>>> - †do
>>> + †while (! (done1 || done2))
>>> † †{
>>> - † † †SVN_ERR(svn_io_file_read_full2(file1_h, buf1,
>>> - † † † † † † † † † † † † † † † † † † SVN__STREAM_CHUNK_SIZE,
>>> &bytes_read1,
>>> - † † † † † † † † † † † † † † † † † † TRUE, pool));
>>> - † † †SVN_ERR(svn_io_file_read_full2(file2_h, buf2,
>>> - † † † † † † † † † † † † † † † † † † SVN__STREAM_CHUNK_SIZE,
>>> &bytes_read2,
>>> - † † † † † † † † † † † † † † † † † † TRUE, pool));
>>> + † † †err = svn_io_file_read_full(file1_h, buf1,
>>> + † † † † † † † † † † † † † † † † †SVN__STREAM_CHUNK_SIZE, &bytes_read1,
>>> pool);
>>>
>>
>> Stefan,
>> This call, and the one following, needed to revert back to
>> svn_io_file_read_full(), since read_full2() was being stuck in an
>> infinite loop. †Perhaps you can investigate?
>>
>
> Hm. That doesn't make any sense. As soon as it tries to read beyond EOF,
> the while (bytes_read1==SVN__STREAM_CHUNK_SIZE) condition
> should be triggered. Looking through the respective APR code for Windows
> and Unix, they always seem to set the number of bytes read even if errors
> were encountered.
>
> How did you trigger the infinite loop?

Change those two function calls, and then run 'make check'. It
stalled infinitely[1], with both cores pegged. Using gdb to attach to
the running process showed that it was svn_io_file_read_full2(),
invoked from that location.

-Hyrum

[1] Well, I've no way of knowing if it was infinite or just taking a
long time, but I'll defer a solution to the Halting Problem as an
exercise for the reader. :)
Received on 2010-09-10 01:45:51 CEST

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