Patchwork revlog: document that mmap resources are released implicitly by GC

login
register
mail settings
Submitter Yuya Nishihara
Date Jan. 13, 2019, 6:44 a.m.
Message ID <eb6ba8bc27e0f8cc2826.1547361893@mimosa>
Download mbox | patch
Permalink /patch/37707/
State Accepted
Headers show

Comments

Yuya Nishihara - Jan. 13, 2019, 6:44 a.m.
# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 1547358986 -32400
#      Sun Jan 13 14:56:26 2019 +0900
# Node ID eb6ba8bc27e0f8cc28268d6601b97f2c2a63c1c3
# Parent  81fb0d015830b5ddf89b398aa63e0b56501dc6ac
revlog: document that mmap resources are released implicitly by GC

It's okay-ish, but currently the open fd and the mapping itself are leaked
until the indexdata is deallocated. If revlog had close(), the underlying
resources should be closed there as well, but AFAIK there's no such hook
point.
Augie Fackler - Jan. 18, 2019, 9:50 a.m.
On Sun, Jan 13, 2019 at 03:44:53PM +0900, Yuya Nishihara wrote:
> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1547358986 -32400
> #      Sun Jan 13 14:56:26 2019 +0900
> # Node ID eb6ba8bc27e0f8cc28268d6601b97f2c2a63c1c3
> # Parent  81fb0d015830b5ddf89b398aa63e0b56501dc6ac
> revlog: document that mmap resources are released implicitly by GC

queued, thanks

Patch

diff --git a/mercurial/revlog.py b/mercurial/revlog.py
--- a/mercurial/revlog.py
+++ b/mercurial/revlog.py
@@ -440,6 +440,8 @@  class revlog(object):
             with self._indexfp() as f:
                 if (mmapindexthreshold is not None and
                     self.opener.fstat(f).st_size >= mmapindexthreshold):
+                    # TODO: should .close() to release resources without
+                    # relying on Python GC
                     indexdata = util.buffer(util.mmapread(f))
                 else:
                     indexdata = f.read()