Merge pull request #1453 from contour-terminal/fix/dead_lock

christianparpart · web-flow · commit 7c6092025bd0 · 2024-02-04T18:18:42.000+01:00
Fix dead lock of terminal
diff --git a/metainfo.xml b/metainfo.xml
@@ -107,7 +107,7 @@
     <release version="0.4.3" urgency="medium" type="development">
       <description>
         <ul>
-          <li> ... </li>
+          <li>Fixes dead lock bug on mouse selection.</li>
         </ul>
       </description>
     </release>
diff --git a/src/contour/TerminalSession.cpp b/src/contour/TerminalSession.cpp
@@ -954,17 +954,18 @@ bool TerminalSession::operator()(actions::ClearHistoryAndReset)
 
 bool TerminalSession::operator()(actions::CopyPreviousMarkRange)
 {
-    copyToClipboard(terminal().extractLastMarkRange());
+    crispy::locked(_terminal, [&]() { copyToClipboard(terminal().extractLastMarkRange()); });
     return true;
 }
 
 bool TerminalSession::operator()(actions::CopySelection copySelection)
 {
+
     switch (copySelection.format)
     {
         case actions::CopyFormat::Text:
             // Copy the selection in pure text, plus whitespaces and newline.
-            copyToClipboard(terminal().extractSelectionText());
+            crispy::locked(_terminal, [&]() { copyToClipboard(terminal().extractSelectionText()); });
             break;
         case actions::CopyFormat::HTML:
             // TODO: This requires walking through each selected cell and construct HTML+CSS for it.
@@ -1098,7 +1099,9 @@ bool TerminalSession::operator()(actions::OpenFileManager)
 
 bool TerminalSession::operator()(actions::OpenSelection)
 {
-    QDesktopServices::openUrl(QUrl(QString::fromUtf8(terminal().extractSelectionText().c_str())));
+    crispy::locked(_terminal, [&]() {
+        QDesktopServices::openUrl(QUrl(QString::fromUtf8(terminal().extractSelectionText().c_str())));
+    });
     return true;
 }
 
diff --git a/src/vtbackend/Terminal.cpp b/src/vtbackend/Terminal.cpp
@@ -1253,8 +1253,6 @@ namespace
 
 string Terminal::extractSelectionText() const
 {
-    auto const _ = std::scoped_lock { *this };
-
     if (!_selection || _selection->state() == Selection::State::Waiting)
         return "";
 
@@ -1274,8 +1272,6 @@ string Terminal::extractSelectionText() const
 
 string Terminal::extractLastMarkRange() const
 {
-    auto const _ = std::lock_guard { *this };
-
     // -1 because we always want to start extracting one line above the cursor by default.
     auto const bottomLine =
         _currentScreen->cursor().position.line + LineOffset(-1) + _settings.copyLastMarkRangeOffset;
diff --git a/src/vtbackend/Terminal.h b/src/vtbackend/Terminal.h
@@ -391,17 +391,9 @@ class Terminal
     void updateInputMethodPreeditString(std::string preeditString);
     // }}}
 
-    void lock() const
-    {
-        _outerLock.lock();
-        _innerLock.lock();
-    }
+    void lock() const { _outerLock.lock(); }
 
-    void unlock() const
-    {
-        _outerLock.unlock();
-        _innerLock.unlock();
-    }
+    void unlock() const { _outerLock.unlock(); }
 
     [[nodiscard]] ColorPalette const& colorPalette() const noexcept { return _state.colorPalette; }
     [[nodiscard]] ColorPalette& colorPalette() noexcept { return _state.colorPalette; }
@@ -810,7 +802,6 @@ class Terminal
 
     // synchronization
     std::mutex mutable _outerLock;
-    std::mutex mutable _innerLock;
 
     // terminal clock
     std::chrono::steady_clock::time_point _currentTime;

Original file line number	Diff line number	Diff line change
`@@ -954,17 +954,18 @@ bool TerminalSession::operator()(actions::ClearHistoryAndReset)`
`954`	`954`
`955`	`955`	`bool TerminalSession::operator()(actions::CopyPreviousMarkRange)`
`956`	`956`	`{`
`957`		`- copyToClipboard(terminal().extractLastMarkRange());`
	`957`	`+ crispy::locked(_terminal, [&]() { copyToClipboard(terminal().extractLastMarkRange()); });`
`958`	`958`	`return true;`
`959`	`959`	`}`
`960`	`960`
`961`	`961`	`bool TerminalSession::operator()(actions::CopySelection copySelection)`
`962`	`962`	`{`
	`963`	`+`
`963`	`964`	`switch (copySelection.format)`
`964`	`965`	`{`
`965`	`966`	`case actions::CopyFormat::Text:`
`966`	`967`	`// Copy the selection in pure text, plus whitespaces and newline.`
`967`		`- copyToClipboard(terminal().extractSelectionText());`
	`968`	`+ crispy::locked(_terminal, [&]() { copyToClipboard(terminal().extractSelectionText()); });`
`968`	`969`	`break;`
`969`	`970`	`case actions::CopyFormat::HTML:`
`970`	`971`	`// TODO: This requires walking through each selected cell and construct HTML+CSS for it.`
`@@ -1098,7 +1099,9 @@ bool TerminalSession::operator()(actions::OpenFileManager)`
`1098`	`1099`
`1099`	`1100`	`bool TerminalSession::operator()(actions::OpenSelection)`
`1100`	`1101`	`{`
`1101`		`- QDesktopServices::openUrl(QUrl(QString::fromUtf8(terminal().extractSelectionText().c_str())));`
	`1102`	`+ crispy::locked(_terminal, [&]() {`
	`1103`	`+ QDesktopServices::openUrl(QUrl(QString::fromUtf8(terminal().extractSelectionText().c_str())));`
	`1104`	`+ });`
`1102`	`1105`	`return true;`
`1103`	`1106`	`}`
`1104`	`1107`
Original file line number	Diff line number	Diff line change
`@@ -1253,8 +1253,6 @@ namespace`
`1253`	`1253`
`1254`	`1254`	`string Terminal::extractSelectionText() const`
`1255`	`1255`	`{`
`1256`		`- auto const _ = std::scoped_lock { *this };`
`1257`		`-`
`1258`	`1256`	`if (!_selection \|\| _selection->state() == Selection::State::Waiting)`
`1259`	`1257`	`return "";`
`1260`	`1258`
`@@ -1274,8 +1272,6 @@ string Terminal::extractSelectionText() const`
`1274`	`1272`
`1275`	`1273`	`string Terminal::extractLastMarkRange() const`
`1276`	`1274`	`{`
`1277`		`- auto const _ = std::lock_guard { *this };`
`1278`		`-`
`1279`	`1275`	`// -1 because we always want to start extracting one line above the cursor by default.`
`1280`	`1276`	`auto const bottomLine =`
`1281`	`1277`	`_currentScreen->cursor().position.line + LineOffset(-1) + _settings.copyLastMarkRangeOffset;`