Skip to content

Commit 19f8e11

Browse files
benlangmuirbenlangmuir
authored
Merge pull request #99 from benlangmuir/racey-init
[tsan] Fix a race during processUnitsAndWait()
2 parents 77f4818 + 18e8fd2 commit 19f8e11

File tree

5 files changed

+21
-90
lines changed

5 files changed

+21
-90
lines changed

‎include/IndexStoreDB/Index/IndexSystem.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,11 +50,10 @@ class INDEXSTOREDB_EXPORT IndexSystem {
5050
bool readonly,
5151
bool enableOutOfDateFileWatching,
5252
bool listenToUnitEvents,
53+
bool waitUntilDoneInitializing,
5354
Optional<size_t> initialDBSize,
5455
std::string &Error);
5556

56-
void waitUntilDoneInitializing();
57-
5857
bool isUnitOutOfDate(StringRef unitOutputPath, ArrayRef<StringRef> dirtyFiles);
5958
bool isUnitOutOfDate(StringRef unitOutputPath, llvm::sys::TimePoint<> outOfDateModTime);
6059

‎lib/CIndexStoreDB/CIndexStoreDB.cpp

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -114,12 +114,9 @@ indexstoredb_index_create(const char *storePath, const char *databasePath,
114114
if (auto index =
115115
IndexSystem::create(storePath, databasePath, libProviderObj, delegate,
116116
useExplicitOutputUnits, readonly,
117-
/*enableOutOfDateFileWatching=*/false, listenToUnitEvents,
117+
/*enableOutOfDateFileWatching=*/false, listenToUnitEvents, wait,
118118
llvm::None, errMsg)) {
119119

120-
if (wait)
121-
index->waitUntilDoneInitializing();
122-
123120
return make_object(index);
124121

125122
} else if (error) {

‎lib/Index/IndexDatastore.cpp

Lines changed: 13 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -88,11 +88,6 @@ struct UnitEventInfo {
8888
: kind(kind), name(std::move(name)), isDependency(isDependency) {}
8989
};
9090

91-
struct DoneInitState {
92-
std::atomic<bool> DoneInit{false};
93-
unsigned RemainingInitUnits = 0; // this is not accessed concurrently.
94-
};
95-
9691
struct PollUnitsState {
9792
llvm::sys::Mutex pollMtx;
9893
llvm::StringMap<sys::TimePoint<>> knownUnits;
@@ -108,9 +103,6 @@ class StoreUnitRepo : public std::enable_shared_from_this<StoreUnitRepo> {
108103

109104
std::shared_ptr<FilePathWatcher> PathWatcher;
110105

111-
DoneInitState InitializingState;
112-
dispatch_semaphore_t InitSemaphore;
113-
114106
PollUnitsState pollUnitsState;
115107

116108
mutable llvm::sys::Mutex StateMtx;
@@ -129,22 +121,13 @@ class StoreUnitRepo : public std::enable_shared_from_this<StoreUnitRepo> {
129121
EnableOutOfDateFileWatching(enableOutOfDateFileWatching),
130122
Delegate(std::move(Delegate)),
131123
CanonPathCache(std::move(canonPathCache)) {
132-
InitSemaphore = dispatch_semaphore_create(0);
133-
}
134-
~StoreUnitRepo() {
135-
dispatch_release(InitSemaphore);
136124
}
137125

138126
void onFilesChange(std::vector<UnitEventInfo> evts,
139127
std::shared_ptr<UnitProcessingSession> processSession,
140128
function_ref<void(unsigned)> ReportCompleted,
141129
function_ref<void()> DirectoryDeleted);
142130

143-
void setInitialUnitCount(unsigned count);
144-
void processedInitialUnitCount(unsigned count);
145-
void finishedUnitInitialization();
146-
void waitUntilDoneInitializing();
147-
148131
/// *For Testing* Poll for any changes to units and wait until they have been registered.
149132
void pollForUnitChangesAndWait();
150133

@@ -185,9 +168,9 @@ class IndexDatastoreImpl {
185168
bool readonly,
186169
bool enableOutOfDateFileWatching,
187170
bool listenToUnitEvents,
171+
bool waitUntilDoneInitializing,
188172
std::string &Error);
189173

190-
void waitUntilDoneInitializing();
191174
bool isUnitOutOfDate(StringRef unitOutputPath, ArrayRef<StringRef> dirtyFiles);
192175
bool isUnitOutOfDate(StringRef unitOutputPath, llvm::sys::TimePoint<> outOfDateModTime);
193176
void checkUnitContainingFileIsOutOfDate(StringRef file);
@@ -464,10 +447,6 @@ void StoreUnitRepo::onFilesChange(std::vector<UnitEventInfo> evts,
464447
};
465448
PathWatcher = std::make_shared<FilePathWatcher>(std::move(pathEventsReceiver));
466449
}
467-
468-
if (!InitializingState.DoneInit) {
469-
processedInitialUnitCount(evts.size());
470-
}
471450
}
472451

473452
void StoreUnitRepo::registerUnit(StringRef unitName, std::shared_ptr<UnitProcessingSession> processSession) {
@@ -761,30 +740,6 @@ void StoreUnitRepo::purgeStaleData() {
761740
// IdxStore->purgeStaleRecords(ActiveRecNames);
762741
}
763742

764-
void StoreUnitRepo::setInitialUnitCount(unsigned count) {
765-
InitializingState.RemainingInitUnits = count;
766-
}
767-
768-
void StoreUnitRepo::processedInitialUnitCount(unsigned count) {
769-
assert(!InitializingState.DoneInit);
770-
InitializingState.RemainingInitUnits -= std::min(count, InitializingState.RemainingInitUnits);
771-
if (InitializingState.RemainingInitUnits == 0) {
772-
finishedUnitInitialization();
773-
}
774-
}
775-
776-
void StoreUnitRepo::finishedUnitInitialization() {
777-
assert(!InitializingState.DoneInit);
778-
dispatch_semaphore_signal(InitSemaphore);
779-
InitializingState.DoneInit = true;
780-
}
781-
782-
void StoreUnitRepo::waitUntilDoneInitializing() {
783-
if (InitializingState.DoneInit)
784-
return;
785-
dispatch_semaphore_wait(InitSemaphore, DISPATCH_TIME_FOREVER);
786-
}
787-
788743
void StoreUnitRepo::pollForUnitChangesAndWait() {
789744
sys::ScopedLock L(pollUnitsState.pollMtx);
790745
std::vector<UnitEventInfo> events;
@@ -1081,6 +1036,7 @@ bool IndexDatastoreImpl::init(IndexStoreRef idxStore,
10811036
bool readonly,
10821037
bool enableOutOfDateFileWatching,
10831038
bool listenToUnitEvents,
1039+
bool waitUntilDoneInitializing,
10841040
std::string &Error) {
10851041
this->IdxStore = std::move(idxStore);
10861042
if (!this->IdxStore)
@@ -1092,18 +1048,8 @@ bool IndexDatastoreImpl::init(IndexStoreRef idxStore,
10921048
auto UnitRepo = std::make_shared<StoreUnitRepo>(this->IdxStore, SymIndex, useExplicitOutputUnits, enableOutOfDateFileWatching, Delegate, CanonPathCache);
10931049
std::weak_ptr<StoreUnitRepo> WeakUnitRepo = UnitRepo;
10941050
auto eventsDeque = std::make_shared<UnitEventInfoDeque>();
1095-
auto OnUnitsChange = [WeakUnitRepo, Delegate, eventsDeque](IndexStore::UnitEventNotification EventNote) {
1096-
if (EventNote.isInitial()) {
1097-
auto UnitRepo = WeakUnitRepo.lock();
1098-
if (!UnitRepo)
1099-
return;
1100-
size_t evtCount = EventNote.getEventsCount();
1101-
if (evtCount == 0) {
1102-
UnitRepo->finishedUnitInitialization();
1103-
} else {
1104-
UnitRepo->setInitialUnitCount(evtCount);
1105-
}
1106-
}
1051+
auto OnUnitsChange = [WeakUnitRepo, Delegate, eventsDeque, waitUntilDoneInitializing](IndexStore::UnitEventNotification EventNote) {
1052+
bool shouldWait = waitUntilDoneInitializing && EventNote.isInitial();
11071053

11081054
std::vector<UnitEventInfo> evts;
11091055
for (size_t i = 0, e = EventNote.getEventsCount(); i != e; ++i) {
@@ -1112,25 +1058,23 @@ bool IndexDatastoreImpl::init(IndexStoreRef idxStore,
11121058
}
11131059

11141060
auto session = std::make_shared<UnitProcessingSession>(eventsDeque, WeakUnitRepo, Delegate);
1115-
session->process(std::move(evts), /*waitForProcessing=*/false);
1061+
session->process(std::move(evts), shouldWait);
11161062
};
11171063

1064+
this->UnitRepo = std::move(UnitRepo);
1065+
11181066
if (listenToUnitEvents) {
11191067
this->IdxStore->setUnitEventHandler(OnUnitsChange);
1120-
bool err = this->IdxStore->startEventListening(/*waitInitialSync=*/false, Error);
1068+
bool err = this->IdxStore->startEventListening(waitUntilDoneInitializing, Error);
11211069
if (err)
11221070
return true;
1071+
} else if (waitUntilDoneInitializing) {
1072+
pollForUnitChangesAndWait();
11231073
}
11241074

1125-
this->UnitRepo = std::move(UnitRepo);
11261075
return false;
11271076
}
11281077

1129-
void IndexDatastoreImpl::waitUntilDoneInitializing() {
1130-
if (UnitRepo)
1131-
UnitRepo->waitUntilDoneInitializing();
1132-
}
1133-
11341078
bool IndexDatastoreImpl::isUnitOutOfDate(StringRef unitOutputPath, ArrayRef<StringRef> dirtyFiles) {
11351079
auto mostRecentFileAndTime = UnitMonitor::getMostRecentModTime(dirtyFiles);
11361080
return isUnitOutOfDate(unitOutputPath, mostRecentFileAndTime.second);
@@ -1182,10 +1126,12 @@ IndexDatastore::create(IndexStoreRef idxStore,
11821126
bool readonly,
11831127
bool enableOutOfDateFileWatching,
11841128
bool listenToUnitEvents,
1129+
bool waitUntilDoneInitializing,
11851130
std::string &Error) {
11861131
std::unique_ptr<IndexDatastoreImpl> Impl(new IndexDatastoreImpl());
11871132
bool Err = Impl->init(std::move(idxStore), std::move(SymIndex), std::move(Delegate), std::move(CanonPathCache),
1188-
useExplicitOutputUnits, readonly, enableOutOfDateFileWatching, listenToUnitEvents, Error);
1133+
useExplicitOutputUnits, readonly, enableOutOfDateFileWatching,
1134+
listenToUnitEvents, waitUntilDoneInitializing, Error);
11891135
if (Err)
11901136
return nullptr;
11911137

@@ -1200,10 +1146,6 @@ IndexDatastore::~IndexDatastore() {
12001146
delete IMPL;
12011147
}
12021148

1203-
void IndexDatastore::waitUntilDoneInitializing() {
1204-
return IMPL->waitUntilDoneInitializing();
1205-
}
1206-
12071149
bool IndexDatastore::isUnitOutOfDate(StringRef unitOutputPath, ArrayRef<StringRef> dirtyFiles) {
12081150
return IMPL->isUnitOutOfDate(unitOutputPath, dirtyFiles);
12091151
}

‎lib/Index/IndexDatastore.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,10 +44,9 @@ class IndexDatastore {
4444
bool readonly,
4545
bool enableOutOfDateFileWatching,
4646
bool listenToUnitEvents,
47+
bool waitUntilDoneInitializing,
4748
std::string &Error);
4849

49-
void waitUntilDoneInitializing();
50-
5150
bool isUnitOutOfDate(StringRef unitOutputPath, ArrayRef<StringRef> dirtyFiles);
5251
bool isUnitOutOfDate(StringRef unitOutputPath, llvm::sys::TimePoint<> outOfDateModTime);
5352

‎lib/Index/IndexSystem.cpp

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -119,11 +119,10 @@ class IndexSystemImpl {
119119
std::shared_ptr<IndexSystemDelegate> Delegate,
120120
bool useExplicitOutputUnits, bool readonly,
121121
bool enableOutOfDateFileWatching, bool listenToUnitEvents,
122+
bool waitUntilDoneInitializing,
122123
Optional<size_t> initialDBSize,
123124
std::string &Error);
124125

125-
void waitUntilDoneInitializing();
126-
127126
bool isUnitOutOfDate(StringRef unitOutputPath, ArrayRef<StringRef> dirtyFiles);
128127
bool isUnitOutOfDate(StringRef unitOutputPath, llvm::sys::TimePoint<> outOfDateModTime);
129128
void checkUnitContainingFileIsOutOfDate(StringRef file);
@@ -210,6 +209,7 @@ bool IndexSystemImpl::init(StringRef StorePath,
210209
std::shared_ptr<IndexSystemDelegate> Delegate,
211210
bool useExplicitOutputUnits, bool readonly,
212211
bool enableOutOfDateFileWatching, bool listenToUnitEvents,
212+
bool waitUntilDoneInitializing,
213213
Optional<size_t> initialDBSize,
214214
std::string &Error) {
215215
this->StorePath = StorePath;
@@ -252,17 +252,14 @@ bool IndexSystemImpl::init(StringRef StorePath,
252252
readonly,
253253
enableOutOfDateFileWatching,
254254
listenToUnitEvents,
255+
waitUntilDoneInitializing,
255256
Error);
256257

257258
if (!this->IndexStore)
258259
return true;
259260
return false;
260261
}
261262

262-
void IndexSystemImpl::waitUntilDoneInitializing() {
263-
IndexStore->waitUntilDoneInitializing();
264-
}
265-
266263
bool IndexSystemImpl::isUnitOutOfDate(StringRef unitOutputPath, ArrayRef<StringRef> dirtyFiles) {
267264
return IndexStore->isUnitOutOfDate(unitOutputPath, dirtyFiles);
268265
}
@@ -611,12 +608,13 @@ IndexSystem::create(StringRef StorePath,
611608
std::shared_ptr<IndexSystemDelegate> Delegate,
612609
bool useExplicitOutputUnits, bool readonly,
613610
bool enableOutOfDateFileWatching, bool listenToUnitEvents,
611+
bool waitUntilDoneInitializing,
614612
Optional<size_t> initialDBSize,
615613
std::string &Error) {
616614
std::unique_ptr<IndexSystemImpl> Impl(new IndexSystemImpl());
617615
bool Err = Impl->init(StorePath, dbasePath, std::move(storeLibProvider), std::move(Delegate),
618616
useExplicitOutputUnits, readonly,
619-
enableOutOfDateFileWatching, listenToUnitEvents,
617+
enableOutOfDateFileWatching, listenToUnitEvents, waitUntilDoneInitializing,
620618
initialDBSize, Error);
621619
if (Err)
622620
return nullptr;
@@ -632,10 +630,6 @@ IndexSystem::~IndexSystem() {
632630
delete IMPL;
633631
}
634632

635-
void IndexSystem::waitUntilDoneInitializing() {
636-
return IMPL->waitUntilDoneInitializing();
637-
}
638-
639633
bool IndexSystem::isUnitOutOfDate(StringRef unitOutputPath, ArrayRef<StringRef> dirtyFiles) {
640634
return IMPL->isUnitOutOfDate(unitOutputPath, dirtyFiles);
641635
}

0 commit comments

Comments
 (0)