diff --git a/include/eepp/ui/doc/textdocument.hpp b/include/eepp/ui/doc/textdocument.hpp index 86b30a6cc..ec0f285c5 100644 --- a/include/eepp/ui/doc/textdocument.hpp +++ b/include/eepp/ui/doc/textdocument.hpp @@ -833,7 +833,7 @@ class EE_API TextDocument { size_t mLastSelection{ 0 }; std::unique_ptr mHighlighter; Mutex mStopFlagsMutex; - UnorderedMap> mStopFlags; + UnorderedMap> mStopFlags; FoldRangeService mFoldRangeService; void initializeCommands(); diff --git a/include/eepp/ui/uicodeeditor.hpp b/include/eepp/ui/uicodeeditor.hpp index 9cbb93ae4..46ec1c000 100644 --- a/include/eepp/ui/uicodeeditor.hpp +++ b/include/eepp/ui/uicodeeditor.hpp @@ -960,7 +960,6 @@ class EE_API UICodeEditor : public UITouchDraggableWidget, public TextDocument:: Time mFoldsRefreshTime; Uint32 mTabWidth; Uint32 mLigatureFeatures{ 0 }; - std::atomic mHighlightWordProcessing{ false }; TextRange mLinkPosition; String mLink; Vector2f mScroll; diff --git a/src/eepp/ui/doc/textdocument.cpp b/src/eepp/ui/doc/textdocument.cpp index 8beafcfed..8b37bad97 100644 --- a/src/eepp/ui/doc/textdocument.cpp +++ b/src/eepp/ui/doc/textdocument.cpp @@ -338,8 +338,14 @@ TextDocument::~TextDocument() { mHighlighter->setStopTokenizingAsync(); // TODO: Use a condition variable to wait the thread pool to finish - while ( !mStopFlags.empty() ) + while ( true ) { + { + Lock l( mStopFlagsMutex ); + if ( mStopFlags.empty() ) + break; + } Sys::sleep( Milliseconds( 0.1 ) ); + } if ( mLoading ) { mLoading = false; @@ -3722,7 +3728,7 @@ TextDocument::SearchResult TextDocument::findLast( const String& text, TextPosit void TextDocument::stopActiveFindAll() { Lock l( mStopFlagsMutex ); for ( const auto& stopFlag : mStopFlags ) - *stopFlag.second.get() = true; + stopFlag.second->store( true, std::memory_order_relaxed ); } bool TextDocument::isDoingTextInput() const { @@ -3739,8 +3745,8 @@ TextDocument::SearchResults TextDocument::findAll( const String& text, bool case SearchResults all; TextDocument::SearchResult found; TextPosition from = startOfDoc(); - auto stopFlagUP = std::make_unique( false ); - bool* stopFlag = stopFlagUP.get(); + auto stopFlagUP = std::make_unique( false ); + std::atomic_bool* stopFlag = stopFlagUP.get(); { Lock l( mStopFlagsMutex ); mStopFlags.insert( { stopFlag, std::move( stopFlagUP ) } ); @@ -3755,7 +3761,8 @@ TextDocument::SearchResults TextDocument::findAll( const String& text, bool case break; from = found.result.end(); all.push_back( found ); - if ( ( maxResults != 0 && all.size() >= maxResults ) || *stopFlag ) + if ( ( maxResults != 0 && all.size() >= maxResults ) || + stopFlag->load( std::memory_order_relaxed ) ) break; } } while ( found.isValid() ); diff --git a/src/eepp/ui/uicodeeditor.cpp b/src/eepp/ui/uicodeeditor.cpp index 84db768c1..274c52a1e 100644 --- a/src/eepp/ui/uicodeeditor.cpp +++ b/src/eepp/ui/uicodeeditor.cpp @@ -245,11 +245,6 @@ UICodeEditor::~UICodeEditor() { // Remember to stop all the async find jobs mDoc->stopActiveFindAll(); - // TODO: Use a condition variable to wait the thread pool to finish - // Wait to end all the async find jobs - while ( mHighlightWordProcessing ) - Sys::sleep( Milliseconds( 0.1 ) ); - mDocView.setDocument( nullptr ); std::size_t clientsOfTypeCount = mDoc->clientOfTypeCount( TextDocument::Client::Type::Core ); long useCount = mDoc.use_count(); @@ -3968,6 +3963,16 @@ const TextSearchParams& UICodeEditor::getHighlightWord() const { } void UICodeEditor::updateHighlightWordCache() { + struct HighlightSearchJob { + HighlightSearchJob( std::shared_ptr searchedDocument, + TextSearchParams searchedParams ) : + document( std::move( searchedDocument ) ), params( std::move( searchedParams ) ) {} + + const std::shared_ptr document; + const TextSearchParams params; + TextRanges ranges; + }; + if ( mHighlightWord.isEmpty() ) return; @@ -3976,33 +3981,43 @@ void UICodeEditor::updateHighlightWordCache() { removeActionsByTag( tag ); runOnMainThread( [this, tag]() { - getUISceneNode()->getThreadPool()->removeWithTag( tag ); - getUISceneNode()->getThreadPool()->run( - [this]() { - if ( mDoc->isRunningTransaction() ) + auto threadPool = getUISceneNode()->getThreadPool(); + threadPool->removeWithTag( tag ); + auto search = std::make_shared( mDoc, mHighlightWord ); + const auto lifetime = mAsyncLifetime.weakHandle(); + threadPool->run( + [search, lifetime]() { + if ( search->document->isRunningTransaction() ) return; Clock docSearch; - mHighlightWordProcessing++; - mDoc->stopActiveFindAll(); + search->document->stopActiveFindAll(); - auto wordCache = mDoc->findAll( - mHighlightWord.escapeSequences ? String::unescape( mHighlightWord.text ) - : mHighlightWord.text, - mHighlightWord.caseSensitive, mHighlightWord.wholeWord, - mHighlightWord.type, mHighlightWord.range ); - - { - Lock l( mHighlightWordCacheMutex ); - mHighlightWordCache = wordCache.ranges(); - } + const String searchedText = search->params.escapeSequences + ? String::unescape( search->params.text ) + : search->params.text; + search->ranges = search->document + ->findAll( searchedText, search->params.caseSensitive, + search->params.wholeWord, + search->params.type, search->params.range ) + .ranges(); Log::info( "Document search triggered in document: \"%s\", searched for " "\"%s\" and took %.2f ms", - mDoc->getFilename().c_str(), - mHighlightWord.text.toUtf8().c_str(), + search->document->getFilename(), search->params.text.toUtf8(), docSearch.getElapsedTime().asMilliseconds() ); + + lifetime.run( [search]( UICodeEditor* editor ) { + if ( editor->mDoc != search->document || + editor->mHighlightWord != search->params ) + return; + { + Lock l( editor->mHighlightWordCacheMutex ); + editor->mHighlightWordCache = std::move( search->ranges ); + } + editor->invalidateDraw(); + } ); }, - [this]( const auto& ) { mHighlightWordProcessing--; }, tag ); + {}, tag ); }, Milliseconds( 16 ), tag ); } else { diff --git a/src/tests/unit_tests/textdocument_tests.cpp b/src/tests/unit_tests/textdocument_tests.cpp index a3da9537c..b51a6d2f1 100644 --- a/src/tests/unit_tests/textdocument_tests.cpp +++ b/src/tests/unit_tests/textdocument_tests.cpp @@ -1,8 +1,10 @@ #include "utest.hpp" #include +#include #include #include #include +#include using namespace EE::UI::Doc; using namespace EE::System; @@ -171,6 +173,38 @@ UTEST( TextDocument, insertLargeMultilineBlock ) { EXPECT_STRINGEQ( "tail\n", doc.line( insertedLineCount + 2 ).getText() ); } +UTEST( TextDocument, findAllCanBeCancelledConcurrently ) { + constexpr size_t lineCount = 65536; + String text; + text.reserve( lineCount * 48 ); + for ( size_t i = 0; i < lineCount; ++i ) + text.append( "needle xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx\n" ); + + TextDocument doc; + doc.textInput( text ); + TextDocument::SearchResults results; + std::atomic_bool started{ false }; + std::atomic_bool finished{ false }; + std::thread worker( [&] { + started.store( true, std::memory_order_release ); + results = doc.findAll( "needle" ); + finished.store( true, std::memory_order_release ); + } ); + + while ( !started.load( std::memory_order_acquire ) ) + Sys::sleep( Milliseconds( 0.1 ) ); + while ( !finished.load( std::memory_order_acquire ) ) { + doc.stopActiveFindAll(); + Sys::sleep( Milliseconds( 0.1 ) ); + } + worker.join(); + + EXPECT_TRUE( results.size() <= lineCount ); + EXPECT_TRUE( results.empty() || results.isSorted() ); + for ( const auto& result : results ) + EXPECT_TRUE( result.isValid() ); +} + UTEST( TextDocument, multilineInsertCursorEndsBeforeExistingSuffix ) { TextDocument doc; doc.insert( 0, { 0, 0 }, "prefix-suffix" ); diff --git a/src/tests/unit_tests/uicodeeditor_tests.cpp b/src/tests/unit_tests/uicodeeditor_tests.cpp index 7c25f6a54..1a9483e8c 100644 --- a/src/tests/unit_tests/uicodeeditor_tests.cpp +++ b/src/tests/unit_tests/uicodeeditor_tests.cpp @@ -1,4 +1,5 @@ -#include "utest.h" +#include "utest.hpp" +#include #include #include #include @@ -23,8 +24,48 @@ class TestableCodeEditor : public UICodeEditor { bool isLongestLineWidthDirtyForTest() const { return mLongestLineWidthDirty; } void clearLongestLineWidthDirtyForTest() { mLongestLineWidthDirty = false; } + + TextRanges getHighlightWordCacheForTest() { + Lock l( mHighlightWordCacheMutex ); + return mHighlightWordCache; + } + + void clearHighlightWordCacheForTest() { + Lock l( mHighlightWordCacheMutex ); + mHighlightWordCache.clear(); + } }; +template +static bool waitForCondition( Predicate&& predicate, const Time& timeout = Seconds( 10 ) ) { + Clock clock; + while ( !predicate() && clock.getElapsedTime() < timeout ) + Sys::sleep( Milliseconds( 1 ) ); + return predicate(); +} + +static bool waitForThreadPool( const std::shared_ptr& threadPool ) { + auto completed = std::make_shared( false ); + threadPool->run( [completed] { completed->store( true, std::memory_order_release ); } ); + return waitForCondition( [completed] { return completed->load( std::memory_order_acquire ); } ); +} + +static void dispatchDebouncedEditorWork() { + Sys::sleep( Milliseconds( 20 ) ); + SceneManager::instance()->update(); +} + +static String makeLargeMarkdownDocument() { + constexpr size_t blockCount = 16384; + String text; + text.reserve( blockCount * 80 ); + for ( size_t i = 0; i < blockCount; ++i ) { + text.append( "## Heading\nInline `code` and ``span``.\n```cpp\nvalue\n```\n" ); + } + text.append( "unique-final-token\n" ); + return text; +} + UTEST( SyntaxColorScheme, CopiesShareStorageAndDetachOnMutation ) { auto defaults = SyntaxColorScheme::getDefaultDark(); auto copy = defaults; @@ -133,6 +174,87 @@ UTEST( UICodeEditor, DefersLongestLineMeasurementForLargeChanges ) { eeDelete( editor ); } +UTEST( UICodeEditor, AsyncHighlightRejectsStaleResultsAfterLargePaste ) { + UIApplication app( WindowSettings{ 320, 240, "eepp - async highlight test" } ); + auto threadPool = ThreadPool::createShared( 1 ); + app.getUI()->setThreadPool( threadPool ); + auto* editor = eeNew( TestableCodeEditor, () ); + editor->setParent( app.getUI()->getRoot() ); + editor->getDocument().textInput( makeLargeMarkdownDocument() ); + + editor->clearHighlightWordCacheForTest(); + editor->setHighlightWord( { "`" } ); + dispatchDebouncedEditorWork(); + ASSERT_TRUE( waitForThreadPool( threadPool ) ); + + // The old result is already queued for the main thread. Changing the query before pumping that + // callback must make the result stale and leave the cache untouched. + editor->setHighlightWord( { "unique-final-token" } ); + SceneManager::instance()->update(); + EXPECT_TRUE( editor->getHighlightWordCacheForTest().empty() ); + + dispatchDebouncedEditorWork(); + ASSERT_TRUE( waitForThreadPool( threadPool ) ); + SceneManager::instance()->update(); + auto ranges = editor->getHighlightWordCacheForTest(); + ASSERT_EQ( size_t{ 1 }, ranges.size() ); + + // Reproduce the #956 workload: a fresh multiline paste followed by rapid literal backtick + // queries while an earlier highlight scan can still be publishing its result. + for ( size_t i = 0; i < 4; ++i ) { + editor->setHighlightWord( { "`" } ); + dispatchDebouncedEditorWork(); + editor->setHighlightWord( { "``" } ); + editor->setHighlightWord( { "```" } ); + dispatchDebouncedEditorWork(); + ASSERT_TRUE( waitForThreadPool( threadPool ) ); + SceneManager::instance()->update(); + } + + EXPECT_STDSTREQ( "```", editor->getHighlightWord().text.toUtf8() ); + auto expected = editor->getDocument().findAll( "```" ).ranges(); + EXPECT_TRUE( expected == editor->getHighlightWordCacheForTest() ); + + eeDelete( editor ); + app.getUI()->setThreadPool( nullptr ); + threadPool.reset(); +} + +UTEST( UICodeEditor, AsyncHighlightSurvivesEditorDestruction ) { + UIApplication app( WindowSettings{ 320, 240, "eepp - async highlight destruction test" } ); + auto threadPool = ThreadPool::createShared( 1 ); + app.getUI()->setThreadPool( threadPool ); + std::atomic_bool blockerStarted{ false }; + std::atomic_bool releaseBlocker{ false }; + threadPool->run( [&] { + blockerStarted.store( true, std::memory_order_release ); + while ( !releaseBlocker.load( std::memory_order_acquire ) ) + Sys::sleep( Milliseconds( 1 ) ); + } ); + const bool started = + waitForCondition( [&] { return blockerStarted.load( std::memory_order_acquire ); } ); + if ( !started ) + releaseBlocker.store( true, std::memory_order_release ); + ASSERT_TRUE( started ); + + auto* editor = eeNew( TestableCodeEditor, () ); + editor->setParent( app.getUI()->getRoot() ); + editor->getDocument().textInput( makeLargeMarkdownDocument() ); + + editor->setHighlightWord( { "missing-highlight-value" } ); + dispatchDebouncedEditorWork(); + const Uint64 tag = reinterpret_cast( editor ); + const bool searchQueued = threadPool->existsTagInQueue( tag ); + releaseBlocker.store( true, std::memory_order_release ); + ASSERT_TRUE( searchQueued ); + ASSERT_TRUE( waitForCondition( [&] { return !threadPool->existsTagInQueue( tag ); } ) ); + eeDelete( editor ); + + app.getUI()->setThreadPool( nullptr ); + threadPool.reset(); + SceneManager::instance()->update(); +} + UTEST( UICodeEditor, DefaultKeybindingCacheTracksConfiguredModifiers ) { const Uint32 originalDefaultModifier = KeyMod::getDefaultModifier(); const Uint32 originalSecondaryModifier = KeyMod::getDefaultSecondaryModifier();