Skip to content

Commit

Permalink
[Core] Fix a case where we could fail to detect a cycle.
Browse files Browse the repository at this point in the history
 - In rare circumstances (which unfortunately I have not been able to reduce a
   sensible test case for), we could get in a situation where we failed to
   detect a cycle, because we were not considering *all* of the scan records,
   just the ones accessible via a task.

 - https://bugs.swift.org/browse/SR-1948
  • Loading branch information
ddunbar committed Oct 16, 2016
1 parent e563e2a commit e6baec4
Showing 1 changed file with 22 additions and 6 deletions.
28 changes: 22 additions & 6 deletions lib/Core/BuildEngine.cpp
Expand Up @@ -187,6 +187,10 @@ class BuildEngineImpl {
assert(isScanning());
return inProgressInfo.pendingScanRecord;
}
const RuleScanRecord* getPendingScanRecord() const {
assert(isScanning());
return inProgressInfo.pendingScanRecord;
}
void setPendingScanRecord(RuleScanRecord* value) {
inProgressInfo.pendingScanRecord = value;
}
Expand All @@ -195,6 +199,10 @@ class BuildEngineImpl {
assert(isInProgress());
return inProgressInfo.pendingTaskInfo;
}
const TaskInfo* getPendingTaskInfo() const {
assert(isInProgress());
return inProgressInfo.pendingTaskInfo;
}
void setPendingTaskInfo(TaskInfo* value) {
assert(isInProgress());
inProgressInfo.pendingTaskInfo = value;
Expand Down Expand Up @@ -869,7 +877,7 @@ class BuildEngineImpl {

// Gather all of the successor relationships.
std::unordered_map<Rule*, std::vector<Rule*>> graph;
std::vector<RuleScanRecord *> activeRuleScanRecords;
std::vector<const RuleScanRecord *> activeRuleScanRecords;
for (const auto& it: taskInfos) {
const TaskInfo& taskInfo = it.second;
assert(taskInfo.forRuleInfo);
Expand All @@ -881,14 +889,22 @@ class BuildEngineImpl {
for (const auto& request: taskInfo.deferredScanRequests) {
// Add the sucessor for the deferred relationship itself.
successors.push_back(&request.ruleInfo->rule);

// Add the active rule scan record which needs to be traversed.
assert(request.ruleInfo->isScanning());
activeRuleScanRecords.push_back(
request.ruleInfo->getPendingScanRecord());
}
graph.insert({ &taskInfo.forRuleInfo->rule, successors });
}

// Add the pending scan records for every rule.
//
// NOTE: There is a very subtle condition around this versus adding the ones
// accessible via the tasks, see https://bugs.swift.org/browse/SR-1948.
// Unfortunately, we do not have a test case for this!
for (const auto& it: ruleInfos) {
const RuleInfo& ruleInfo = it.second;
if (ruleInfo.isScanning()) {
const auto* scanRecord = ruleInfo.getPendingScanRecord();
activeRuleScanRecords.push_back(scanRecord);
}
}

// Gather dependencies from all of the active scan records.
std::unordered_set<const RuleScanRecord*> visitedRuleScanRecords;
Expand Down

0 comments on commit e6baec4

Please sign in to comment.