Issue #1783 resolved

thg keeps a ref to a folderrepo for too long ?

Johan Samyn
created an issue

Hi,

Today I created a small test repo, added it to the reporegistry and opened it. When I was finished with it, I closzed the repotab, and even removed it from the reporegistry. Then I opened Windows Explorer and tried to delete the repo folder. That however presented an eror message. See the attachment for a screendump. Happens both on Win 7 pro 32 and 64 bit. Only when I closed the Workbench I was able to delete the folder. I guess there is some reference to that repo folder staying behind, even when it is removed from the reporegistry, causing some lock on the folder ?

Johan

Comments (18)

  1. Eric Siegerman

    Confirmed that thg holds the handles:

    1. I launched thg, and opened repository testrepo. ProcessExplorer showed the thg process holding three handles to various places in testrepo/.hg -- and no other handles to anything in testrepo.
    2. I closed testrepo in thg (there doesn't appear to be a menu option, but CTRL-W appears to do the trick). At that point, ProcessExplorer showed (what appear to be) the same three handles as before.
    3. When I quit thg, the handles went away.

    See attached screen snaps.

  2. Yuya Nishihara
    • changed status to open

    First, thgrepo module keeps repo objects in _repocache until terminating the process. This means repository monitoring won't stop.

    On Windows, QFileSystemWatcher uses FindFirstChangeNotification API, which looks to obtain file handle. And Windows doesn't allow to remove directory containing open files.

    Maybe we need to clean up _repocache when repository closed, and preferably shutdown repository monitoring explicitly.

  3. Johan Samyn reporter

    Yuya, for clarity: you agree we should be cleaning up _repocache only when a repo is _removed_ from the reporegistry (not when it is closed) ?

    And I see the following 'complications':

    • If there are subrepos for that repo, I suppose we should deal with them too, recursively.
    • If we remove a repotreegroup we should perform the necessary steps recursively for all elements of it (subgroups, repos in the group or in its subgroups, all their subrepos, ...).

    Next point: am I right that this solution should be added to the removeRows() method of the RepoTreeItem class in repotreeitem.py ?

    Last point: should we continue to discuss further details of the solution here in the issue itself, or would it be better to do that via the thg-dev mailing list ?

  4. Yuya Nishihara

    cleaning up _repocache only when a repo is _removed_ from the reporegistry (not when it is closed) ?

    I didn't think deeply, but I meant when it gets unused by any widgets. But as you know, it will be quite hard to implement. Also I'm not sure that repo object can be garbage-collected just by removing from _repocache.

    Another possible solution, I think, is to enable/disable file monitoring when the repository opened/closed by workbench.

    this solution should be added to the removeRows() method of the RepoTreeItem class in repotreeitem.py ?

    It will assist removal of unused repo objects, but won't fix the root cause of #1248 and this #1783. I do not see this issue as a reporegistry's job.

    here in the issue itself, or would it be better to do that via the thg-dev mailing list

    If it goes on an implementation detail, I think mailing list is preferable.

  5. Yuya Nishihara

    Idea:

    • If no receiver of xxxChanged signal exists, that repository would be unused
    • Check unused state soon after RepoWidget tab is closed
    • Stop filesystem watcher of unused repo (and remove it from _repocache if possible)
  6. Yuya Nishihara

    thgrepo: add stub to manage open repositories and bundle signals (refs #1783)

    It will be used by QtRunner and Workbench in order to keep open repositories, stop filesystem monitoring of closed repositories, and listen on signals from multiple repositories.

    Currently it only implements mapping of several repository signals and basic refcount.

    → <<cset bfb825b06f46>>

  7. Log in to comment