Skip to content

[tsan] Fix a race during processUnitsAndWait() #99

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 12, 2020
Merged

[tsan] Fix a race during processUnitsAndWait() #99

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions include/IndexStoreDB/Index/IndexSystem.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,10 @@ class INDEXSTOREDB_EXPORT IndexSystem {
bool readonly,
bool enableOutOfDateFileWatching,
bool listenToUnitEvents,
bool waitUntilDoneInitializing,
Optional<size_t> initialDBSize,
std::string &Error);

void waitUntilDoneInitializing();

bool isUnitOutOfDate(StringRef unitOutputPath, ArrayRef<StringRef> dirtyFiles);
bool isUnitOutOfDate(StringRef unitOutputPath, llvm::sys::TimePoint<> outOfDateModTime);

Expand Down
5 changes: 1 addition & 4 deletions lib/CIndexStoreDB/CIndexStoreDB.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,12 +114,9 @@ indexstoredb_index_create(const char *storePath, const char *databasePath,
if (auto index =
IndexSystem::create(storePath, databasePath, libProviderObj, delegate,
useExplicitOutputUnits, readonly,
/*enableOutOfDateFileWatching=*/false, listenToUnitEvents,
/*enableOutOfDateFileWatching=*/false, listenToUnitEvents, wait,
llvm::None, errMsg)) {

if (wait)
index->waitUntilDoneInitializing();

return make_object(index);

} else if (error) {
Expand Down
84 changes: 13 additions & 71 deletions lib/Index/IndexDatastore.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,11 +88,6 @@ struct UnitEventInfo {
: kind(kind), name(std::move(name)), isDependency(isDependency) {}
};

struct DoneInitState {
std::atomic<bool> DoneInit{false};
unsigned RemainingInitUnits = 0; // this is not accessed concurrently.
};

struct PollUnitsState {
llvm::sys::Mutex pollMtx;
llvm::StringMap<sys::TimePoint<>> knownUnits;
Expand All @@ -108,9 +103,6 @@ class StoreUnitRepo : public std::enable_shared_from_this<StoreUnitRepo> {

std::shared_ptr<FilePathWatcher> PathWatcher;

DoneInitState InitializingState;
dispatch_semaphore_t InitSemaphore;

PollUnitsState pollUnitsState;

mutable llvm::sys::Mutex StateMtx;
Expand All @@ -129,22 +121,13 @@ class StoreUnitRepo : public std::enable_shared_from_this<StoreUnitRepo> {
EnableOutOfDateFileWatching(enableOutOfDateFileWatching),
Delegate(std::move(Delegate)),
CanonPathCache(std::move(canonPathCache)) {
InitSemaphore = dispatch_semaphore_create(0);
}
~StoreUnitRepo() {
dispatch_release(InitSemaphore);
}

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

void setInitialUnitCount(unsigned count);
void processedInitialUnitCount(unsigned count);
void finishedUnitInitialization();
void waitUntilDoneInitializing();

/// *For Testing* Poll for any changes to units and wait until they have been registered.
void pollForUnitChangesAndWait();

Expand Down Expand Up @@ -185,9 +168,9 @@ class IndexDatastoreImpl {
bool readonly,
bool enableOutOfDateFileWatching,
bool listenToUnitEvents,
bool waitUntilDoneInitializing,
std::string &Error);

void waitUntilDoneInitializing();
bool isUnitOutOfDate(StringRef unitOutputPath, ArrayRef<StringRef> dirtyFiles);
bool isUnitOutOfDate(StringRef unitOutputPath, llvm::sys::TimePoint<> outOfDateModTime);
void checkUnitContainingFileIsOutOfDate(StringRef file);
Expand Down Expand Up @@ -464,10 +447,6 @@ void StoreUnitRepo::onFilesChange(std::vector<UnitEventInfo> evts,
};
PathWatcher = std::make_shared<FilePathWatcher>(std::move(pathEventsReceiver));
}

if (!InitializingState.DoneInit) {
processedInitialUnitCount(evts.size());
}
}

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

void StoreUnitRepo::setInitialUnitCount(unsigned count) {
InitializingState.RemainingInitUnits = count;
}

void StoreUnitRepo::processedInitialUnitCount(unsigned count) {
assert(!InitializingState.DoneInit);
InitializingState.RemainingInitUnits -= std::min(count, InitializingState.RemainingInitUnits);
if (InitializingState.RemainingInitUnits == 0) {
finishedUnitInitialization();
}
}

void StoreUnitRepo::finishedUnitInitialization() {
assert(!InitializingState.DoneInit);
dispatch_semaphore_signal(InitSemaphore);
InitializingState.DoneInit = true;
}

void StoreUnitRepo::waitUntilDoneInitializing() {
if (InitializingState.DoneInit)
return;
dispatch_semaphore_wait(InitSemaphore, DISPATCH_TIME_FOREVER);
}

void StoreUnitRepo::pollForUnitChangesAndWait() {
sys::ScopedLock L(pollUnitsState.pollMtx);
std::vector<UnitEventInfo> events;
Expand Down Expand Up @@ -1081,6 +1036,7 @@ bool IndexDatastoreImpl::init(IndexStoreRef idxStore,
bool readonly,
bool enableOutOfDateFileWatching,
bool listenToUnitEvents,
bool waitUntilDoneInitializing,
std::string &Error) {
this->IdxStore = std::move(idxStore);
if (!this->IdxStore)
Expand All @@ -1092,18 +1048,8 @@ bool IndexDatastoreImpl::init(IndexStoreRef idxStore,
auto UnitRepo = std::make_shared<StoreUnitRepo>(this->IdxStore, SymIndex, useExplicitOutputUnits, enableOutOfDateFileWatching, Delegate, CanonPathCache);
std::weak_ptr<StoreUnitRepo> WeakUnitRepo = UnitRepo;
auto eventsDeque = std::make_shared<UnitEventInfoDeque>();
auto OnUnitsChange = [WeakUnitRepo, Delegate, eventsDeque](IndexStore::UnitEventNotification EventNote) {
if (EventNote.isInitial()) {
auto UnitRepo = WeakUnitRepo.lock();
if (!UnitRepo)
return;
size_t evtCount = EventNote.getEventsCount();
if (evtCount == 0) {
UnitRepo->finishedUnitInitialization();
} else {
UnitRepo->setInitialUnitCount(evtCount);
}
}
auto OnUnitsChange = [WeakUnitRepo, Delegate, eventsDeque, waitUntilDoneInitializing](IndexStore::UnitEventNotification EventNote) {
bool shouldWait = waitUntilDoneInitializing && EventNote.isInitial();

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

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

this->UnitRepo = std::move(UnitRepo);

if (listenToUnitEvents) {
this->IdxStore->setUnitEventHandler(OnUnitsChange);
bool err = this->IdxStore->startEventListening(/*waitInitialSync=*/false, Error);
bool err = this->IdxStore->startEventListening(waitUntilDoneInitializing, Error);
if (err)
return true;
} else if (waitUntilDoneInitializing) {
pollForUnitChangesAndWait();
}

this->UnitRepo = std::move(UnitRepo);
return false;
}

void IndexDatastoreImpl::waitUntilDoneInitializing() {
if (UnitRepo)
UnitRepo->waitUntilDoneInitializing();
}

bool IndexDatastoreImpl::isUnitOutOfDate(StringRef unitOutputPath, ArrayRef<StringRef> dirtyFiles) {
auto mostRecentFileAndTime = UnitMonitor::getMostRecentModTime(dirtyFiles);
return isUnitOutOfDate(unitOutputPath, mostRecentFileAndTime.second);
Expand Down Expand Up @@ -1182,10 +1126,12 @@ IndexDatastore::create(IndexStoreRef idxStore,
bool readonly,
bool enableOutOfDateFileWatching,
bool listenToUnitEvents,
bool waitUntilDoneInitializing,
std::string &Error) {
std::unique_ptr<IndexDatastoreImpl> Impl(new IndexDatastoreImpl());
bool Err = Impl->init(std::move(idxStore), std::move(SymIndex), std::move(Delegate), std::move(CanonPathCache),
useExplicitOutputUnits, readonly, enableOutOfDateFileWatching, listenToUnitEvents, Error);
useExplicitOutputUnits, readonly, enableOutOfDateFileWatching,
listenToUnitEvents, waitUntilDoneInitializing, Error);
if (Err)
return nullptr;

Expand All @@ -1200,10 +1146,6 @@ IndexDatastore::~IndexDatastore() {
delete IMPL;
}

void IndexDatastore::waitUntilDoneInitializing() {
return IMPL->waitUntilDoneInitializing();
}

bool IndexDatastore::isUnitOutOfDate(StringRef unitOutputPath, ArrayRef<StringRef> dirtyFiles) {
return IMPL->isUnitOutOfDate(unitOutputPath, dirtyFiles);
}
Expand Down
3 changes: 1 addition & 2 deletions lib/Index/IndexDatastore.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,9 @@ class IndexDatastore {
bool readonly,
bool enableOutOfDateFileWatching,
bool listenToUnitEvents,
bool waitUntilDoneInitializing,
std::string &Error);

void waitUntilDoneInitializing();

bool isUnitOutOfDate(StringRef unitOutputPath, ArrayRef<StringRef> dirtyFiles);
bool isUnitOutOfDate(StringRef unitOutputPath, llvm::sys::TimePoint<> outOfDateModTime);

Expand Down
16 changes: 5 additions & 11 deletions lib/Index/IndexSystem.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,11 +119,10 @@ class IndexSystemImpl {
std::shared_ptr<IndexSystemDelegate> Delegate,
bool useExplicitOutputUnits, bool readonly,
bool enableOutOfDateFileWatching, bool listenToUnitEvents,
bool waitUntilDoneInitializing,
Optional<size_t> initialDBSize,
std::string &Error);

void waitUntilDoneInitializing();

bool isUnitOutOfDate(StringRef unitOutputPath, ArrayRef<StringRef> dirtyFiles);
bool isUnitOutOfDate(StringRef unitOutputPath, llvm::sys::TimePoint<> outOfDateModTime);
void checkUnitContainingFileIsOutOfDate(StringRef file);
Expand Down Expand Up @@ -210,6 +209,7 @@ bool IndexSystemImpl::init(StringRef StorePath,
std::shared_ptr<IndexSystemDelegate> Delegate,
bool useExplicitOutputUnits, bool readonly,
bool enableOutOfDateFileWatching, bool listenToUnitEvents,
bool waitUntilDoneInitializing,
Optional<size_t> initialDBSize,
std::string &Error) {
this->StorePath = StorePath;
Expand Down Expand Up @@ -252,17 +252,14 @@ bool IndexSystemImpl::init(StringRef StorePath,
readonly,
enableOutOfDateFileWatching,
listenToUnitEvents,
waitUntilDoneInitializing,
Error);

if (!this->IndexStore)
return true;
return false;
}

void IndexSystemImpl::waitUntilDoneInitializing() {
IndexStore->waitUntilDoneInitializing();
}

bool IndexSystemImpl::isUnitOutOfDate(StringRef unitOutputPath, ArrayRef<StringRef> dirtyFiles) {
return IndexStore->isUnitOutOfDate(unitOutputPath, dirtyFiles);
}
Expand Down Expand Up @@ -611,12 +608,13 @@ IndexSystem::create(StringRef StorePath,
std::shared_ptr<IndexSystemDelegate> Delegate,
bool useExplicitOutputUnits, bool readonly,
bool enableOutOfDateFileWatching, bool listenToUnitEvents,
bool waitUntilDoneInitializing,
Optional<size_t> initialDBSize,
std::string &Error) {
std::unique_ptr<IndexSystemImpl> Impl(new IndexSystemImpl());
bool Err = Impl->init(StorePath, dbasePath, std::move(storeLibProvider), std::move(Delegate),
useExplicitOutputUnits, readonly,
enableOutOfDateFileWatching, listenToUnitEvents,
enableOutOfDateFileWatching, listenToUnitEvents, waitUntilDoneInitializing,
initialDBSize, Error);
if (Err)
return nullptr;
Expand All @@ -632,10 +630,6 @@ IndexSystem::~IndexSystem() {
delete IMPL;
}

void IndexSystem::waitUntilDoneInitializing() {
return IMPL->waitUntilDoneInitializing();
}

bool IndexSystem::isUnitOutOfDate(StringRef unitOutputPath, ArrayRef<StringRef> dirtyFiles) {
return IMPL->isUnitOutOfDate(unitOutputPath, dirtyFiles);
}
Expand Down