Comment 14 for bug 495023

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Gareth White wrote:
> Hi Andrew,
>
> I'm afraid I can still reproduce the issue on OS X even with your patch.
> So there must be something else contributing to the problem here.
>
> I noticed that in _dirstate_helpers_pyx.c it appears that
> __pyx_f_6bzrlib_21_dirstate_helpers_pyx_13ProcessEntryC__update_current_block()
> is not propagating any exceptions that occur - instead it's calling
> __Pyx_WriteUnraisable() and returning.
>
> According to
> http://www.cosc.canterbury.ac.nz/greg.ewing/python/Pyrex/version/Doc/Manual/basics.html#mozTocId482761
> this is the expected behaviour for "cdef void" functions unless you
> declare them with "except *".
>
> Could this help explain the behaviour I'm seeing?

I would think so. Specifically, this code path is the 'walk to next
directory' function. If an exception were raised, then likely the
current block pointer would get out of sync.

My guess is that the filesystem pointer would be at the next directory,
but the internal walker would still be at the current dir.

Going further, now that they are out of sync, the internal walker will
think that all of these directories have been removed, since they are
not found. (we walk the filesystem and internal pointer in lock-step,
and can tell if something has gone missing, or was added, etc.)

My guess is that we are seeing this happen in this particular location,
because 'osutils.is_inside()' is a pure-python function. And thus the
existing exception is 'trapped' and not raised until we get to this
point, at which point we see KeyboardInterrupt get raised, and it
confuses us here.

It doesn't look like the function has to be 'void', we could just do:

=== modified file 'bzrlib/_dirstate_helpers_pyx.pyx'
- --- bzrlib/_dirstate_helpers_pyx.pyx 2009-10-02 05:43:41 +0000
+++ bzrlib/_dirstate_helpers_pyx.pyx 2010-01-04 18:37:21 +0000
@@ -1392,7 +1392,7 @@
             # provide.
             self.search_specific_file_parents.add('')

- - cdef void _update_current_block(self):
+ cdef int _update_current_block(self) except -1:
         if (self.block_index < len(self.state._dirblocks) and
             osutils.is_inside(self.current_root,
self.state._dirblocks[self.block_index][0])):
             self.current_block = self.state._dirblocks[self.block_index]
@@ -1401,6 +1401,7 @@
         else:
             self.current_block = None
             self.current_block_list = None
+ return 0

     def __next__(self):
         # Simple thunk to allow tail recursion without pyrex confusion

And then the exception should get propagated, the walkers won't get out
of sync, and instead it will just cancel the current operation.

John
=:->

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAktCNYgACgkQJdeBCYSNAANqIgCeJlMizlBoRT2HNsHUZ+nLSXfi
0IAAn2SK+Pjr26TorP76dg55vORnmfyu
=GQZd
-----END PGP SIGNATURE-----