Skip to content

Commit c962310

Browse files
committed
Guard against empty port values
This fixes a segfault. Also fixes a re-entrant `processEvents()` call. Signed-off-by: Patrick Avery <patrick.avery@kitware.com>
1 parent f2628eb commit c962310

7 files changed

Lines changed: 37 additions & 4 deletions

tomviz/ProgressDialogManager.cxx

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,14 @@ void ProgressDialogManager::onNodeExecutionStarted(pipeline::Node* node)
160160
dialog->adjustSize();
161161
dialog->resize(500, dialog->height());
162162
dialog->show();
163-
QCoreApplication::processEvents();
163+
dialog->raise();
164+
// NOTE: deliberately no QCoreApplication::processEvents() here. This slot
165+
// runs while handling a nodeExecutionStarted event, and pumping the event
166+
// loop would re-entrantly deliver other already-queued executor signals
167+
// (e.g. executionComplete -> Pipeline::executionFinished), firing finish
168+
// handlers mid-execution while upstream ports are still empty -- a crash.
169+
// The pipeline runs on a worker thread (ThreadedExecutor), so the main loop
170+
// is not blocked and the dialog paints on the next event-loop iteration.
164171
}
165172

166173
void ProgressDialogManager::onNodeExecutionFinished(pipeline::Node* node,

tomviz/pipeline/GatedEditorWidget.cxx

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -95,19 +95,28 @@ void GatedEditorWidget::onExecutionFinished()
9595
return;
9696
}
9797

98+
// Build the inner editor first. The factory can legitimately return null if
99+
// the upstream data isn't actually usable yet (e.g. this finished signal was
100+
// delivered re-entrantly mid-execution); in that case stay in the not-ready
101+
// state rather than tearing down the widget and showing a blank editor.
102+
if (!buildInner()) {
103+
m_notReadyWidget->setRunEnabled(true);
104+
return;
105+
}
106+
98107
m_layout->removeWidget(m_notReadyWidget);
99108
m_notReadyWidget->deleteLater();
100109
m_notReadyWidget = nullptr;
101-
buildInner();
102110
}
103111

104-
void GatedEditorWidget::buildInner()
112+
bool GatedEditorWidget::buildInner()
105113
{
106114
m_inner = m_factory(this);
107115
if (m_inner) {
108116
m_layout->addWidget(m_inner, 1);
109117
}
110118
emit canApplyChanged();
119+
return m_inner != nullptr;
111120
}
112121

113122
} // namespace pipeline

tomviz/pipeline/GatedEditorWidget.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ class GatedEditorWidget : public EditNodeWidget
4848
void onRunRequested();
4949
void onExecutionFinished();
5050
bool inputsReady() const;
51-
void buildInner();
51+
bool buildInner();
5252

5353
Node* m_node;
5454
Pipeline* m_pipeline;

tomviz/pipeline/transforms/ArrayWranglerTransform.cxx

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,9 @@ EditNodeWidget* ArrayWranglerTransform::createPropertiesWidget(
166166
this, pipeline,
167167
[this](QWidget* p) -> EditNodeWidget* {
168168
auto vol = inputPorts()[0]->data().value<VolumeDataPtr>();
169+
if (!vol || !vol->imageData()) {
170+
return nullptr;
171+
}
169172
return new ArrayWranglerWidget(this, vol, p);
170173
},
171174
parent);

tomviz/pipeline/transforms/CropTransform.cxx

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,9 @@ EditNodeWidget* CropTransform::createPropertiesWidget(Pipeline* pipeline,
126126
this, pipeline,
127127
[this](QWidget* p) -> EditNodeWidget* {
128128
auto vol = inputPorts()[0]->data().value<VolumeDataPtr>();
129+
if (!vol || !vol->imageData()) {
130+
return nullptr;
131+
}
129132
return new CropWidget(this, vol, p);
130133
},
131134
parent);

tomviz/pipeline/transforms/SetTiltAnglesTransform.cxx

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -467,6 +467,14 @@ EditNodeWidget* SetTiltAnglesTransform::createPropertiesWidget(
467467
this, pipeline,
468468
[this](QWidget* p) -> EditNodeWidget* {
469469
auto vol = inputPorts()[0]->data().value<VolumeDataPtr>();
470+
// Guard against an empty/mismatched port value. value<T>() is
471+
// non-throwing and yields a null pointer when the upstream data is not
472+
// (yet) a usable volume; constructing the widget would then dereference
473+
// it. Returning nullptr lets GatedEditorWidget stay in its not-ready
474+
// state instead of crashing.
475+
if (!vol || !vol->imageData()) {
476+
return nullptr;
477+
}
470478
return new SetTiltAnglesWidget(this, vol, p);
471479
},
472480
parent);

tomviz/pipeline/transforms/TranslateAlignTransform.cxx

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,9 @@ EditNodeWidget* TranslateAlignTransform::createPropertiesWidget(
9595
this, pipeline,
9696
[this](QWidget* p) -> EditNodeWidget* {
9797
auto vol = inputPorts()[0]->data().value<VolumeDataPtr>();
98+
if (!vol || !vol->imageData()) {
99+
return nullptr;
100+
}
98101
vtkSmartPointer<vtkImageData> imageData(vol->imageData());
99102
return new AlignWidget(this, imageData, p);
100103
},

0 commit comments

Comments
 (0)