On Thu, Sep 9, 2010 at 4:34 PM, Stefan Fuhrmann <stefanfuhrm...@alice-dsl.de> wrote: > Hyrum Wright wrote: >> >> On Thu, Sep 9, 2010 at 10:59 AM, <hwri...@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. :)
