Mercurial > hgsubversion

comparison hgsubversion/maps.py @ 1550:67b28d657f62

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
sqliterevmap: break ".hashes()" cycle in a safer way The `fromsvn()` revset implementation could cause weakref error when using sqliterevmap like: File "hgsubversion/util.py", line 357, in <lambda> return subset.filter(lambda r: tonode(r) in hashes) File "hgsubversion/maps.py", line 542, in __contains__ return self.get(key) != None File "hgsubversion/maps.py", line 533, in get for row in self.revmap._query( ReferenceError: weakly-referenced object no longer exists Basically the seemingly harmless assignment could break surprisingly: # dangerous: `hashes` does not have a reference of `meta.revmap` and may # become unavailable after `meta`, `revmap` being released by refcount. hashes = meta.revmap.hashes() The above syntax is nice to support while avoiding cycles is also nice. This patch removes `revmap._hashes` so the revmap no longer owns a reference of a `ReverseRevMap` object so the `ReverseRevMap` object no longer needs to use weakref for `self.revmap`. This could actually be caught by `comprehensive/test_sqlite_revmap.py`. I was not careful enough verifying the "fromsvn()" patch.
author Jun Wu <quark@fb.com>
date Thu, 21 Dec 2017 17:39:52 -0800
parents 09476d758b59
children cff81f35b31e
comparison
equal deleted inserted replaced
1549:8410a978c650 1550:67b28d657f62
521 sqlblobtype = bytes if sys.version_info >= (3, 0) else buffer 521 sqlblobtype = bytes if sys.version_info >= (3, 0) else buffer
522 522
523 class ReverseRevMap(object): 523 class ReverseRevMap(object):
524 # collections.Mapping is not suitable since we don't want 2/3 of 524 # collections.Mapping is not suitable since we don't want 2/3 of
525 # its required interfaces: __iter__, __len__. 525 # its required interfaces: __iter__, __len__.
526 def __init__(self, revmap): 526 def __init__(self, revmap, cache):
527 self.revmap = weakref.proxy(revmap) 527 self.revmap = revmap
528 self._cache = {} 528 self._cache = cache
529 529
530 def get(self, key, default=None): 530 def get(self, key, default=None):
531 if key not in self._cache: 531 if key not in self._cache:
532 result = None 532 result = None
533 for row in self.revmap._query( 533 for row in self.revmap._query(
563 self._dbpath = revmap_path + '.db' 563 self._dbpath = revmap_path + '.db'
564 self._rowcountpath = self._dbpath + '.rowcount' 564 self._rowcountpath = self._dbpath + '.rowcount'
565 self._lastpulledpath = lastpulled_path 565 self._lastpulledpath = lastpulled_path
566 566
567 self._db = None 567 self._db = None
568 self._hashes = None
569 self._sqlitepragmas = sqlitepragmas 568 self._sqlitepragmas = sqlitepragmas
570 self.firstpulled = 0 569 self.firstpulled = 0
571 self._updatefirstlastpulled() 570 self._updatefirstlastpulled()
572 # __iter__ is expensive and thus disabled by default 571 # __iter__ is expensive and thus disabled by default
573 # it should only be enabled for testing 572 # it should only be enabled for testing
574 self._allowiter = False 573 self._allowiter = False
574 # used by self.hashes(), {hash: (rev, branch)}
575 self._hashescache = {}
575 576
576 def hashes(self): 577 def hashes(self):
577 if self._hashes is None: 578 return self.ReverseRevMap(self, self._hashescache)
578 self._hashes = self.ReverseRevMap(self)
579 return self._hashes
580 579
581 def branchedits(self, branch, revnum): 580 def branchedits(self, branch, revnum):
582 return [((r[0], r[1] or None), bytes(r[2])) for r in 581 return [((r[0], r[1] or None), bytes(r[2])) for r in
583 self._query('SELECT rev, branch, hash FROM revmap ' + 582 self._query('SELECT rev, branch, hash FROM revmap ' +
584 'WHERE rev < ? AND branch = ? ' + 583 'WHERE rev < ? AND branch = ? ' +
607 def clear(self): 606 def clear(self):
608 hgutil.unlinkpath(self._filepath, ignoremissing=True) 607 hgutil.unlinkpath(self._filepath, ignoremissing=True)
609 hgutil.unlinkpath(self._dbpath, ignoremissing=True) 608 hgutil.unlinkpath(self._dbpath, ignoremissing=True)
610 hgutil.unlinkpath(self._rowcountpath, ignoremissing=True) 609 hgutil.unlinkpath(self._rowcountpath, ignoremissing=True)
611 self._db = None 610 self._db = None
612 self._hashes = None 611 self._hashescache = {}
613 self._firstpull = None 612 self._firstpull = None
614 self._lastpull = None 613 self._lastpull = None
615 614
616 def batchset(self, items, lastpulled): 615 def batchset(self, items, lastpulled):
617 with self._transaction(): 616 with self._transaction():
644 self._insert([(revnum, branch, binha)]) 643 self._insert([(revnum, branch, binha)])
645 if revnum < self.firstpulled or not self.firstpulled: 644 if revnum < self.firstpulled or not self.firstpulled:
646 self.firstpulled = revnum 645 self.firstpulled = revnum
647 if revnum > self.lastpulled or not self.lastpulled: 646 if revnum > self.lastpulled or not self.lastpulled:
648 self.lastpulled = revnum 647 self.lastpulled = revnum
649 if self._hashes is not None: 648 if self._hashescache:
650 self._hashes._cache[binha] = key 649 self._hashescache[binha] = key
651 650
652 def __delitem__(self, key): 651 def __delitem__(self, key):
653 for row in self._querybykey('DELETE', key): 652 for row in self._querybykey('DELETE', key):
654 if self.rowcount > 0: 653 if self.rowcount > 0:
655 self.rowcount -= 1 654 self.rowcount -= 1
656 return 655 return
657 # For performance reason, self._hashes is not updated 656 # For performance reason, self._hashescache is not updated
658 raise KeyError(key) 657 raise KeyError(key)
659 658
660 @contextlib.contextmanager 659 @contextlib.contextmanager
661 def _transaction(self, mode='IMMEDIATE'): 660 def _transaction(self, mode='IMMEDIATE'):
662 if self._db is None: 661 if self._db is None: