On Mon, Jan 20, 2014 at 3:12 PM, Evgeny Kotkov
<evgeny.kotkov_at_visualsvn.com>wrote:
> Hi,
>
> I've noticed that the new fs-fs-pack-tests (#13 and #14) fail on my machine
> around trunk_at_r1559108. This happens with different Windows versions (I've
> tried Windows XP SP3 32-bit / Vista SP2 32-bit / Server 2008 R2 64-bit /
> Windows 7 64-bit / Windows 8 64-bit) and does not happen with Ubuntu
> 64-bit:
> [[[
> svn_tests: E720145: Can't remove directory
> 'upgrade_new_txns_to_log_addressing\revs\2': The directory is not empty.
> FAIL: fs-fs-pack-test.exe 13: upgrade txns to log addressing in shared
> FSFS
>
> svn_tests: E720145: Can't remove directory
> 'upgrade_old_txns_to_log_addressing\revs\2': The directory is not empty.
> FAIL: fs-fs-pack-test.exe 14: upgrade txns started before svnadmin
> upgrade
> ]]]
>
> This Windows-specific error is usually caused by an attempt to remove a
> folder
> while still having open file handles to some of its children. So, I
> grabbed
> the Process Monitor [1] and dumped the CreateFile and CloseFile events that
> happen within the 'upgrade_new_txns_to_log_addressing' test. It is easy to
> spot that the 'revs/2/8' file handle is only closed during the test cleanup
> (when the per-function TEST_POOL is being cleared). As a consequence, an
> attempt to pack the repository fails to delete the 'revs/2' folder with an
> ENOTEMPTY error. I've tracked down the origin of this file handle and here
> is the corresponding stacktrace:
> [[[
> file_open + 0x20, libsvn_subr\io.c(350)
> svn_io_file_open + 0x4a, libsvn_subr\io.c(3427)
> open_pack_or_rev_file + 0x48, libsvn_fs_fs\rev_file.c(67)
> svn_fs_fs__open_pack_or_rev_file + 0x45, libsvn_fs_fs\rev_file.c(124)
> open_and_seek_revision + 0x52, libsvn_fs_fs\cached_data.c(217)
> get_node_revision_body + 0x1a7, libsvn_fs_fs\cached_data.c(338)
> svn_fs_fs__get_node_revision + 0x2a, libsvn_fs_fs\cached_data.c(387)
> shards_spanned + 0x7a, libsvn_fs_fs\transaction.c(1858)
> choose_delta_base + 0x8e, libsvn_fs_fs\transaction.c(1925)
> write_container_delta_rep + 0xd5, libsvn_fs_fs\transaction.c(2655)
> write_final_rev + 0x22b, libsvn_fs_fs\transaction.c(2923)
> commit_body + 0x344, libsvn_fs_fs\transaction.c(3843)
> ...
> ]]]
>
> Now, if you examine the core FSFS routines like 'get_node_revision_body',
> you
> can notice that some of them close the temporary
> 'svn_fs_fs__revision_file_t'
> objects, but some of them do not, e.g., svn_fs_fs__get_changes does, but
> get_node_revision_body does not (for the physical addressing mode). In the
> latter case, this can lead to various sorts of problems, one of which was
> caught by the fs-fs-pack-tests.
>
> This behavior looks like a regression from 1.8.x, where these routines did
> close the temporary file handles via explicit 'svn_io_file_close' calls or
> via
> less explicit 'svn_stream_close' calls for the streams with DISOWN=FALSE.
> I've attached a patch that restores this behavior for FSFS7. The invariant
> is quite simple: on any non-erroneous flow, a routine should not leak the
> file
> handle(s) in the POOL that come from the temporary
> 'svn_fs_fs__revision_file_t'
> object(s).
>
> NOTE: It looks like FSX suffers from the same kind of problem. There are
> 77
> failing tests from the standalone suite (client-test.exe, fs-test.exe,
> ...) and
> any attempt to run the Python tests (basic_tests.py, ...) immediately fails
> during the greek tree initialization phase. I guess I might be able to
> come up
> with a patch for this problem in the nearby future (in case someone does
> not
> fix it earlier):
>
> [[[
> (client-test.exe --fs-type=fsx)
>
> svn_tests: E720145: Can't remove directory
> 'BUILDPATH\subversion\tests\cmdline\svn-test-work\libsvn_client\
> test-wc-add-repos\db\transactions\0-0.txn': The directory is not empty.
> FAIL: client-test.exe 3: test svn_wc_add3 scenarios
>
> (win-tests.py --fs-type=fsx --test=basic_tests.py --log-to-stdout)
>
> Testing Release configuration on local repository.
> START: basic_tests.py
> E: import did not succeed, while creating greek repos.
> E: The final line from 'svn import' was:
> E: Can't remove directory
> 'BUILDPATH\subversion\tests\cmdline\svn-test-work\local-tmp\
> repos\db\transactions\0-0.txn': The directory is not empty.
> ]]]
>
> [1] http://technet.microsoft.com/sysinternals/bb896645
>
>
> Thanks and regards,
> Evgeny Kotkov
>
Hi Evgeny,
Thanks for the patch! Reviewed and committed as r1559777.
-- Stefan^2.
P.S.: Ugh, Windows! ;)
Received on 2014-01-20 18:16:24 CET