Skip to content

Commit

Permalink
Fix race condition when saving selection to file (nickbnf#234)
Browse files Browse the repository at this point in the history
  • Loading branch information
@variar
variar committed Dec 23, 2020
1 parent 2a696eb commit 4c80e46
Showing 1 changed file with 46 additions and 37 deletions.
83 changes: 46 additions & 37 deletions src/ui/src/abstractlogview.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -502,7 +502,7 @@ void AbstractLogView::keyPressEvent( QKeyEvent* keyEvent )
const auto altModifier = keyEvent->modifiers().testFlag( Qt::AltModifier );
const auto noModifier = keyEvent->modifiers() == Qt::NoModifier;

const auto jumpToBottomLine = [this]() {
const auto jumpToBottomLine = [ this ]() {
disableFollow();
const auto line = LineNumber( logData->getNbLine().get() ) - 1_lcount;
selection_.selectLine( line );
Expand Down Expand Up @@ -1058,7 +1058,7 @@ void AbstractLogView::saveToFile()
return;
}

QProgressDialog progressDialog;
QProgressDialog progressDialog( this );
progressDialog.setLabelText( QString( "Saving content to %1" ).arg( filename ) );

std::vector<std::pair<LineNumber, LinesCount>> offsets;
Expand All @@ -1078,19 +1078,21 @@ void AbstractLogView::saveToFile()

AtomicFlag interruptRequest;

progressDialog.setRange( 0, static_cast<int>( offsets.size() ) );
progressDialog.setRange( 0, static_cast<int>( offsets.size() + 1 ) );
connect( &progressDialog, &QProgressDialog::canceled,
[&interruptRequest]() { interruptRequest.set(); } );
[ &interruptRequest ]() { interruptRequest.set(); } );

tbb::flow::graph saveFileGraph;
auto lineReader = tbb::flow::source_node<std::vector<QString>>(
using LinesData = std::pair<std::vector<QString>, bool>;
auto lineReader = tbb::flow::source_node<LinesData>(
saveFileGraph,
[this, &offsets, &interruptRequest, &progressDialog,
offsetIndex = 0u]( std::vector<QString>& lines ) mutable -> bool {
[ this, &offsets, &interruptRequest, &progressDialog,
offsetIndex = 0u ]( LinesData& lines ) mutable -> bool {
if ( !interruptRequest && offsetIndex < offsets.size() ) {
const auto& offset = offsets.at( offsetIndex );
lines = logData->getLines( offset.first, offset.second );
for ( auto& l : lines ) {
lines.first = logData->getLines( offset.first, offset.second );
lines.second = true;
for ( auto& l : lines.first ) {
#if !defined( Q_OS_WIN )
l.append( QChar::CarriageReturn );
#endif
Expand All @@ -1102,16 +1104,28 @@ void AbstractLogView::saveToFile()
return true;
}
else {
progressDialog.finished( static_cast<int>( offsetIndex ) );
lines.second = false;

return false;
}
},
false );

auto lineWriter = tbb::flow::function_node<std::vector<QString>, tbb::flow::continue_msg>(
auto lineWriter = tbb::flow::function_node<LinesData, tbb::flow::continue_msg>(
saveFileGraph, 1,
[&interruptRequest, &codec, &saveFile]( const std::vector<QString>& lines ) {
for ( const auto& l : lines ) {
[ &interruptRequest, &codec, &saveFile, &progressDialog,
linesCount = 0u ]( const LinesData& lines ) mutable {
if ( !lines.second ) {
if ( !interruptRequest ) {
saveFile.commit();
linesCount++;
}

progressDialog.finished( static_cast<int>( linesCount ) );
return tbb::flow::continue_msg{};
}

for ( const auto& l : lines.first ) {

const auto encodedLine = codec->fromUnicode( l );
const auto written = saveFile.write( encodedLine );
Expand All @@ -1121,24 +1135,19 @@ void AbstractLogView::saveToFile()
interruptRequest.set();
return tbb::flow::continue_msg{};
}

linesCount++;
}
return tbb::flow::continue_msg{};
} );

tbb::flow::make_edge( lineReader, lineWriter );

lineReader.activate();

progressDialog.exec();
progressDialog.setWindowModality( Qt::ApplicationModal );
progressDialog.open();

lineReader.activate();
saveFileGraph.wait_for_all();

if ( !interruptRequest ) {
saveFile.commit();
}
else {
LOG( logERROR ) << "Saving file write failed";
}
}

void AbstractLogView::updateSearchLimits()
Expand Down Expand Up @@ -1416,7 +1425,7 @@ void AbstractLogView::jumpToEndOfLine()

// Search the longest line in the selection
const auto max_length = std::accumulate(
selection.cbegin(), selection.cend(), 0_length, [this]( auto currentMax, auto line ) {
selection.cbegin(), selection.cend(), 0_length, [ this ]( auto currentMax, auto line ) {
return qMax( currentMax, logData->getLineLength( LineNumber( line ) ) );
} );
horizontalScrollBar()->setValue( max_length.get() - getNbVisibleCols() );
Expand All @@ -1436,7 +1445,7 @@ void AbstractLogView::jumpToRightOfScreen()

std::transform(
visibleLinesNumbers.begin(), visibleLinesNumbers.end(), std::back_inserter( lineLengths ),
[this]( const auto& line ) { return logData->getLineLength( LineNumber( line ) ); } );
[ this ]( const auto& line ) { return logData->getLineLength( LineNumber( line ) ); } );

const auto max_length = *std::max_element( lineLengths.begin(), lineLengths.end() );
horizontalScrollBar()->setValue( max_length.get() - getNbVisibleCols() );
Expand Down Expand Up @@ -1522,46 +1531,46 @@ void AbstractLogView::createMenu()
{
copyAction_ = new QAction( tr( "&Copy" ), this );
// No text as this action title depends on the type of selection
connect( copyAction_, &QAction::triggered, [this]( auto ) { this->copy(); } );
connect( copyAction_, &QAction::triggered, [ this ]( auto ) { this->copy(); } );

markAction_ = new QAction( tr( "&Mark" ), this );
connect( markAction_, &QAction::triggered, [this]( auto ) { this->markSelected(); } );
connect( markAction_, &QAction::triggered, [ this ]( auto ) { this->markSelected(); } );

saveToFileAction_ = new QAction( tr( "Save to file" ), this );
connect( saveToFileAction_, &QAction::triggered, [this]( auto ) { this->saveToFile(); } );
connect( saveToFileAction_, &QAction::triggered, [ this ]( auto ) { this->saveToFile(); } );

// For '#' and '*', shortcuts doesn't seem to work but
// at least it displays them in the menu, we manually handle those keys
// as keys event anyway (in keyPressEvent).
findNextAction_ = new QAction( tr( "Find &next" ), this );
findNextAction_->setShortcut( Qt::Key_Asterisk );
findNextAction_->setStatusTip( tr( "Find the next occurrence" ) );
connect( findNextAction_, &QAction::triggered, [this]( auto ) { this->findNextSelected(); } );
connect( findNextAction_, &QAction::triggered, [ this ]( auto ) { this->findNextSelected(); } );

findPreviousAction_ = new QAction( tr( "Find &previous" ), this );
findPreviousAction_->setShortcut( tr( "/" ) );
findPreviousAction_->setStatusTip( tr( "Find the previous occurrence" ) );
connect( findPreviousAction_, &QAction::triggered,
[this]( auto ) { this->findPreviousSelected(); } );
[ this ]( auto ) { this->findPreviousSelected(); } );

addToSearchAction_ = new QAction( tr( "&Add to search" ), this );
addToSearchAction_->setStatusTip( tr( "Add the selection to the current search" ) );
connect( addToSearchAction_, &QAction::triggered, [this]( auto ) { this->addToSearch(); } );
connect( addToSearchAction_, &QAction::triggered, [ this ]( auto ) { this->addToSearch(); } );

setSearchStartAction_ = new QAction( tr( "Set search start" ), this );
connect( setSearchStartAction_, &QAction::triggered,
[this]( auto ) { this->setSearchStart(); } );
[ this ]( auto ) { this->setSearchStart(); } );

setSearchEndAction_ = new QAction( tr( "Set search end" ), this );
connect( setSearchEndAction_, &QAction::triggered, [this]( auto ) { this->setSearchEnd(); } );
connect( setSearchEndAction_, &QAction::triggered, [ this ]( auto ) { this->setSearchEnd(); } );

clearSearchLimitAction_ = new QAction( tr( "Clear search limits" ), this );
connect( clearSearchLimitAction_, &QAction::triggered,
[this]( auto ) { this->clearSearchLimits(); } );
[ this ]( auto ) { this->clearSearchLimits(); } );

saveDefaultSplitterSizesAction_ = new QAction( tr( "Save splitter position" ), this );
connect( saveDefaultSplitterSizesAction_, &QAction::triggered,
[this]( auto ) { this->saveDefaultSplitterSizes(); } );
[ this ]( auto ) { this->saveDefaultSplitterSizes(); } );

popupMenu_ = new QMenu( this );
highlightersMenu_ = popupMenu_->addMenu( "Highlighters" );
Expand Down Expand Up @@ -1706,7 +1715,7 @@ void AbstractLogView::drawTextArea( QPaintDevice* paint_device )
leftMarginPx_ = contentStartPosX + SEPARATOR_WIDTH;

const auto searchStartIndex = lineIndex( searchStart_ );
const auto searchEndIndex = [this] {
const auto searchEndIndex = [ this ] {
auto index = lineIndex( searchEnd_ );
if ( searchEnd_ + 1_lcount != displayLineNumber( index ) ) {
// in filtered view lineIndex for "past the end" returns last line
Expand Down Expand Up @@ -1757,7 +1766,7 @@ void AbstractLogView::drawTextArea( QPaintDevice* paint_device )
std::vector<HighlightedMatch> allHighlights;
allHighlights.reserve( highlighterMatches.size() );
std::transform( highlighterMatches.begin(), highlighterMatches.end(),
std::back_inserter( allHighlights ), [&logLine]( const auto& match ) {
std::back_inserter( allHighlights ), [ &logLine ]( const auto& match ) {
const auto prefix = logLine.leftRef( match.startColumn() );
const auto expandedPrefixLength = untabify( prefix ).length();
auto startDelta = expandedPrefixLength - prefix.length();
Expand Down

0 comments on commit 4c80e46

Please sign in to comment.