Re: [ISSUE] Cancelling svn checkout leaves an open file handle

On Tue, Sep 8, 2020 at 6:08 AM Uroš Jovanović <urosh3d_at_gmail.com> wrote:
>
> Then, mid downloading some of the larger files a temp file will appear in .svn\tmp. Once that happens, hit the Cancel button.
> It will signal the cancellation to the svn client and it will throw the SvnOperationCanceledException, the SvnClient gets disposed BUT an open file handle remains on ".svn\tmp\svn-XYZ123" file.
> If you try to delete it, Windows will complain that it is used by our test app. :(

Moving this to the dev@ list...

Potentially long-running APIs such as 'checkout' allow the client to
provide a 'cancel_func' callback, which is called at various strategic
places to ask the client whether the operation should be canceled.

It sounds to me like one of those places sees a cancel request and
returns to its caller, forgetting to do some cleanup.

Last night I tried to find such a place by reading code.

The 'checkout' command sets up a working copy (if necessary) and then
calls the 'update' logic to do the heavy lifting.

The 'update' logic is quite involved as it handles all sorts of
possibilities, which means the number of branches of the call tree
that need to be checked are too numerous for my code reading approach
to be sensible.

My thoughts for an automated approach, provided there is a way for a
process to inquire how many open file handles it has (I assume there
is a way; I've just never had to do this): The idea is to write a
minimal client that does the following (on a ramdrive):

1. Check out a working copy of a repository, giving a cancel_func 'A'
   that increments a global variable 'n' each time it is called and
   always returns "don't cancel."

2. Loop n times, the loop counter being a global variable 'x':

   2.1: Delete the working copy.

   2.2: Check out a working copy of the same repository, giving a
        different cancel_func 'B' that returns "don't cancel" the
        first (x - 1) times it is called, and returns "cancel" the
        x-th time it is called.

   2.3: Test whether there are open file handles. If there are, we
        know at which iteration the cleanup is not done, and we break
        out of the loop.

3. If x >= n, quit; we didn't find the problem.

4. Delete the working copy.

5. Check out a working copy of the same repository, giving a different
   cancel_func 'C' that returns "don't cancel" the first (x - 1) times
   it is called, and traps the x-th time it is called, allowing the
   call stack to be examined.

Notes and caveats:

1. This could run for days (or years).

2. Then again, if it can be exposed pretty reliably by a user hitting
   a Cancel button in a GUI, that means cancel_func is called
   frequently enough from the offending location that it should
   (hopefully) be caught relatively soon in the process.

3. I think a huge repository isn't needed. The Greek Tree used by the
   test suite may suffice. If it doesn't expose the bug, I'd retry
   with a larger file thrown in. If that doesn't expose it, add
   increasing complexity such as externals, etc.

4. This relies on the logic being executed identically for each
   checkout (i.e., cancel_func is called the same number of times from
   the same call sites).

5. No idea how this could be turned into a regression test.

6. If there's a better way, I'd love to hear it!

Nathan
Received on 2020-09-09 06:44:48 CEST