On Wed, 26 Oct 2016 09:42:09 -0700, Durham Goode wrote: > On 10/26/16 5:54 AM, Yuya Nishihara wrote: > > On Tue, 25 Oct 2016 12:46:58 -0700, Durham Goode wrote: > >> On 10/22/16 1:59 AM, Yuya Nishihara wrote: > >>> On Tue, 18 Oct 2016 17:50:16 -0700, Durham Goode wrote: > >>>> # HG changeset patch > >>>> # User Durham Goode <dur...@fb.com> > >>>> # Date 1476837866 25200 > >>>> # Tue Oct 18 17:44:26 2016 -0700 > >>>> # Branch stable > >>>> # Node ID 3efece5c59853908d65de53635488361dbf20c49 > >>>> # Parent ed607426a3ff4deda8c7f2de8b86d5b6ca976d67 > >>>> manifest: make manifestctx store the repo > >>>> > >>>> The old manifestctx stored a reference to the revlog. If the inmemory > >>>> revlog > >>>> became invalid, the ctx now held an old copy and would be incorrect. To > >>>> fix > >>>> this, we need the ctx to go through the manifestlog for each access. > >>>> > >>>> This is the same pattern that changectx already uses (it stores the > >>>> repo, and > >>>> accesses commit data through self._repo.changelog). > >>>> > >>>> diff --git a/mercurial/manifest.py b/mercurial/manifest.py > >>>> --- a/mercurial/manifest.py > >>>> +++ b/mercurial/manifest.py > >>>> @@ -1276,7 +1276,7 @@ class manifestlog(object): > >>>> if self._treeinmem: > >>>> m = treemanifestctx(self._revlog, '', node) > >>>> else: > >>>> - m = manifestctx(self._revlog, node) > >>>> + m = manifestctx(self._repo, node) > >>>> if node != revlog.nullid: > >>>> self._mancache[node] = m > >>> This will create a reference cycle, but I don't know if the situation gets > >>> better or worse after this patch. > >>> > >>> repo -> manifestlog -> _mancache -> manifestctx -> repo > >>> > >>> Is _mancache valid after the manifestlog is recreated? > >> _mancache is not really valid after manifestlog is recreated, since it > >> may contain manifest entries that no longer exist in the file. Even if > >> it didn't contain bad entries, the manifestctx itself needs up-to-date > >> access to the manifest revlog, which is only available through the repo > >> object (since the repo object's property is what handles manifest revlog > >> cache checking). > > Perhaps I miss the point. Do we have stale copies of manifestlog and > > manifestctx > > somewhere? My understanding is we've moved the @storecache to manifestlog() > > at > > 3c8811efdddc, so manifestlog, _revlog (and _mancache) should be accessible > > only > > when they are valid, and manifestlog doesn't live longer than > > repo.manifest(). > Ah, yes manifestctx's may be held for longer than the manifestlog lives, > by code that is out of our control. I'm not aware of specific cases > where this is happening, but it could.
Got it, thanks. It seems manifestctx has slightly different guarantee about its lifetime from changectx. changectx is effectively a snapshot when repo[changeid] is called (more precisely, when propertycached function is called.) _______________________________________________ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel