zuul
/
zuul
Code Issues Proposed changes

Fix race condition relating to change updates 

The gerrit event processing thread performs its own queries and
updates of change objects outside of the main thread, possibly
while those objects are being used in the main thread.  Generally
this is okay because typically only new data are being added to
changes, and if any of the new data are relevant, the main thread
will receive an event about it and update accordingly.

However, in the process of updating changes in the event processing
thread, some attributes of the change may be temporarily set to
incorrect values, such as empty lists.  Switch to atomic updates
of those values so that if the main thread consults the change object,
it has reasonably valid values.

This may have caused sporadic failures of test_dependent_changes_dequeue,
but is also capable of happening in production.

Change-Id: I134d50fe2da8ef344ee8d4fb68a1eed0692b04e2
This commit is contained in:
James E. Blair 2015-05-07 15:19:12 -07:00
parent d765085300
commit 1c46057888
1 changed files with 13 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

23
zuul/trigger/gerrit.py
View File

@ -408,18 +408,19 @@ class Gerrit(object):
change.branch = data['branch']
change.url = data['url']
max_ps = 0
change.files = []
files = []
for ps in data['patchSets']:
if ps['number'] == change.patchset:
change.refspec = ps['ref']
for f in ps.get('files', []):
change.files.append(f['file'])
files.append(f['file'])
if int(ps['number']) > int(max_ps):
max_ps = ps['number']
if max_ps == change.patchset:
change.is_current_patchset = True
else:
change.is_current_patchset = False
change.files = files
change.is_merged = self._isMerged(change)
change.approvals = data['currentPatchSet'].get('approvals', [])
@ -438,7 +439,7 @@ class Gerrit(object):
history = history[:]
history.append(change.number)
change.needs_changes = []
needs_changes = []
if 'dependsOn' in data:
parts = data['dependsOn'][0]['ref'].split('/')
dep_num, dep_ps = parts[3], parts[4]
@ -448,8 +449,8 @@ class Gerrit(object):
self.log.debug("Getting git-dependent change %s,%s" %
(dep_num, dep_ps))
dep = self._getChange(dep_num, dep_ps, history=history)
if (not dep.is_merged) and dep not in change.needs_changes:
change.needs_changes.append(dep)
if (not dep.is_merged) and dep not in needs_changes:
needs_changes.append(dep)
for record in self._getDependsOnFromCommit(data['commitMessage']):
dep_num = record['number']
@ -460,17 +461,18 @@ class Gerrit(object):
self.log.debug("Getting commit-dependent change %s,%s" %
(dep_num, dep_ps))
dep = self._getChange(dep_num, dep_ps, history=history)
if (not dep.is_merged) and dep not in change.needs_changes:
change.needs_changes.append(dep)
if (not dep.is_merged) and dep not in needs_changes:
needs_changes.append(dep)
change.needs_changes = needs_changes
change.needed_by_changes = []
needed_by_changes = []
if 'neededBy' in data:
for needed in data['neededBy']:
parts = needed['ref'].split('/')
dep_num, dep_ps = parts[3], parts[4]
dep = self._getChange(dep_num, dep_ps)
if (not dep.is_merged) and dep.is_current_patchset:
change.needed_by_changes.append(dep)
needed_by_changes.append(dep)
for record in self._getNeededByFromCommit(data['id']):
dep_num = record['number']
@ -483,7 +485,8 @@ class Gerrit(object):
# change).
dep = self._getChange(dep_num, dep_ps, refresh=True)
if (not dep.is_merged) and dep.is_current_patchset:
change.needed_by_changes.append(dep)
needed_by_changes.append(dep)
change.needed_by_changes = needed_by_changes
return change