SymbolicReference.set_reference contains this code, which is meant to attempt a transactional write:

However, because theok flag isinitialized toTrue instead ofFalse--which strongly appears unintentional--it is alwaysTrue even if theok = True statement in thetry block is not reached. Consequently,lfd.rollback() in thefinally block is never called. This seems on first glance like a serious bug, but as detailed below I believe it might actually be quite minor in the context of what kind of cleanup GitPython generallydoes or does not guarantee.

This can be contrasted to another place where that pattern is used correctly:

This is very simple to fix, since it is sufficient to changeTrue toFalse in the initial assignment. So most of the work to fix it would be figuring out where a regression test should go and how it should manage to produce a case where cleanup doesn't happen immediately.

(Besides that simple fix, other fixes that also reduce the risk of such bugs coming back and allow the code to be simplified are possible. In particular, theLockedFD object represents a non-memory resource that always can and should be managed deterministically according to where control flow is, so it ought to implement the context manager protocol.)

I am unsure if this bug causes any problems in practice. If I understandLockedFD correctly, the write that is not rolled back is to a temporary file separate from the real target, accessed through a file object specific to thatLockedFD object. The lock file name is determined from the target file name, so if a separate attempt to write to the target is made, that could cause problems.LockedFD has a__del__ method that should be called eventually and performs the rollback. On CPython, which has reference counting as its primary garbage collection strategy, cleanup would often happen immediately as a result... though not always, even in the absence of reference cycles, because, in exception handling, an exception object holds a reference to a traceback object which holds references to all stack frames in its call stack, through which local variables thus remain accessible until control leaves the handlingexcept block.

Still, it may be that this shall not be considered a bug--or at least not a bug of its own, or at least not a bug that merits a regression test accompanying its bugfix--on the grounds thatit might be considered a special case of theLeakage of System Resources limitation documented prominently in the readme. In support of this view, the third place in the code of GitPython whereldf.rollback is called uses a different pattern and its comments suggest that cleanup viaLockedFD.__del__ is considered acceptable to rely on:

(Interestingly, that is not exactly equivalent to the behavior of thetry-finally pattern used ingit/index/base.py. If it is intended to be, then it should catchBaseException rather thanException.)