From c2fadc3271477e15876d37b2eca28df61e4fe645 Mon Sep 17 00:00:00 2001 From: Ben Langmuir Date: Mon, 23 Jul 2018 14:45:20 -0700 Subject: [PATCH 1/2] [unittest] Fix some unintentional behaviour in EditingTest I haven't found any cases where this changes the visible behaviour of the test, but having the file remain open across invocations was certainly unexpected and affects how the first part of the test works at runtime. --- unittests/SourceKit/SwiftLang/EditingTest.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/unittests/SourceKit/SwiftLang/EditingTest.cpp b/unittests/SourceKit/SwiftLang/EditingTest.cpp index b0ffed709d63f..30aeecad563c2 100644 --- a/unittests/SourceKit/SwiftLang/EditingTest.cpp +++ b/unittests/SourceKit/SwiftLang/EditingTest.cpp @@ -253,7 +253,7 @@ void EditTest::doubleOpenWithDelay(useconds_t delay, bool closeDoc) { // be queried, since the semantic info from the first open is unreachable. for (int i = 0; i < 2; ++i) { bool expired = waitForDocUpdate(/*reset=*/true); - ASSERT_FALSE(expired) << "no second notification"; + ASSERT_FALSE(expired) << "no " << (i ? "second " : "") << "notification"; replaceText(DocName, 0, 0, StringRef(), Consumer); if (!Consumer.Diags.empty()) break; @@ -262,6 +262,8 @@ void EditTest::doubleOpenWithDelay(useconds_t delay, bool closeDoc) { ASSERT_EQ(1u, Consumer.Diags.size()); EXPECT_STREQ("use of unresolved identifier 'unknown_name'", Consumer.Diags[0].Description.c_str()); + + close(DocName); } // This test is failing occassionally in CI: rdar://42483323 From a8d549326ad1a0e62156b79da1f5d89e627d2b13 Mon Sep 17 00:00:00 2001 From: Ben Langmuir Date: Mon, 23 Jul 2018 14:50:09 -0700 Subject: [PATCH 2/2] [sourcekit] Fix a race between AST processing and reopening a document When a document is opened twice without closing it we re-initialize its internal data. Make sure we don't race with code that accesses that data. Found by TSan! --- tools/SourceKit/lib/SwiftLang/SwiftEditor.cpp | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/tools/SourceKit/lib/SwiftLang/SwiftEditor.cpp b/tools/SourceKit/lib/SwiftLang/SwiftEditor.cpp index aebfa42ceb4a4..165337391f4e1 100644 --- a/tools/SourceKit/lib/SwiftLang/SwiftEditor.cpp +++ b/tools/SourceKit/lib/SwiftLang/SwiftEditor.cpp @@ -1673,8 +1673,13 @@ ImmutableTextSnapshotRef SwiftEditorDocument::replaceText( } void SwiftEditorDocument::updateSemaInfo() { - if (Impl.SemanticInfo) { - Impl.SemanticInfo->processLatestSnapshotAsync(Impl.EditableBuffer); + Impl.AccessMtx.lock(); + auto EditableBuffer = Impl.EditableBuffer; + auto SemanticInfo = Impl.SemanticInfo; + Impl.AccessMtx.unlock(); // Not a recursive mutex, so unlock before processing + + if (SemanticInfo) { + SemanticInfo->processLatestSnapshotAsync(EditableBuffer); } } @@ -1978,6 +1983,7 @@ void SwiftEditorDocument::expandPlaceholder(unsigned Offset, unsigned Length, } ImmutableTextSnapshotRef SwiftEditorDocument::getLatestSnapshot() const { + llvm::sys::ScopedLock L(Impl.AccessMtx); return Impl.EditableBuffer->getSnapshot(); }