blackbox: do not cache file objects
Having the blackbox file objects cached in `ui._bbfp` could in theory be
troublesome if multiple processes (ex. chg servers) have file objects
referring to a same file. (Although I spent some time and failed to build a
convincing test case)
This patch makes blackbox re-open the file every time to make the situation
better. Ideally we also need proper locking.
The caching logic traces back to the commit introducing blackbox
(18242716a). That commit does not have details about why caching is
necessary. Consider the fact that blackbox logs are not many, it seems fine
to remove the fp cache to be more confident.
Differential Revision: https://phab.mercurial-scm.org/D650
--- a/hgext/blackbox.py Wed Sep 06 20:54:53 2017 -0700
+++ b/hgext/blackbox.py Wed Sep 06 21:08:59 2017 -0700
@@ -80,7 +80,6 @@
if src is None:
self._partialinit()
else:
- self._bbfp = getattr(src, '_bbfp', None)
self._bbinlog = False
self._bbrepo = getattr(src, '_bbrepo', None)
self._bbvfs = getattr(src, '_bbvfs', None)
@@ -88,7 +87,6 @@
def _partialinit(self):
if util.safehasattr(self, '_bbvfs'):
return
- self._bbfp = None
self._bbinlog = False
self._bbrepo = None
self._bbvfs = None
@@ -143,16 +141,7 @@
if not '*' in self.track and not event in self.track:
return
- if self._bbfp:
- ui = self
- elif self._bbvfs:
- try:
- self._bbfp = self._openlogfile()
- except (IOError, OSError) as err:
- self.debug('warning: cannot write to blackbox.log: %s\n' %
- err.strerror)
- del self._bbvfs
- self._bbfp = None
+ if self._bbvfs:
ui = self
else:
# certain ui instances exist outside the context of
@@ -160,12 +149,12 @@
# was seen.
ui = lastui
- if not ui or not ui._bbfp:
+ if not ui:
return
if not lastui or ui._bbrepo:
lastui = ui
if ui._bbinlog:
- # recursion guard
+ # recursion and failure guard
return
try:
ui._bbinlog = True
@@ -188,19 +177,21 @@
else:
src = ''
try:
- fp = ui._bbfp
fmt = '%s %s @%s%s (%s)%s> %s'
args = (date, user, rev, changed, pid, src, formattedmsg)
- fp.write(fmt % args)
- fp.flush()
- except IOError as err:
+ with ui._openlogfile() as fp:
+ fp.write(fmt % args)
+ except (IOError, OSError) as err:
self.debug('warning: cannot write to blackbox.log: %s\n' %
err.strerror)
+ # do not restore _bbinlog intentionally to avoid failed
+ # logging again
+ else:
+ ui._bbinlog = False
finally:
- ui._bbinlog = False
+ pass
def setrepo(self, repo):
- self._bbfp = None
self._bbinlog = False
self._bbrepo = repo
self._bbvfs = repo.vfs