Repository navigation
Batch: crash fixes from stacktrace triage + defensive hardening - #14849
Merged
Merged
Conversation
… is null synchronize2dIntersectionViews() could crash in two ways: when the view's ownerCase() is null, and when the collection has no Rim3dView ancestor at all (e.g. mid-destruction after detaching from the PDM tree, where firstAncestorOrThisOfTypeAsserted<Rim3dView>() returns null since CAF_ASSERT is compiled out in release). Both cases are now guarded.
Guarded two unconditional null derefs: dockWidget->setWidget(viewWidget) when viewWidget creation fails, and viewPdmObject() in handleViewerDeletion() when no view has ever been assigned to control. Added regression test for the handleViewerDeletion() case; the setWidget() path requires a running GUI/main window and isn't reachable from the headless unit test harness.
…nker RimViewController::masterView() dereferenced ownerViewLinker() without a null check, causing a crash when the controller has been detached from its RimViewLinker parent (e.g. mid-destruction, after the owning child-array field has already erased it but before delete runs). The same unguarded ownerViewLinker() dereference existed in several other methods: isActive(), isCameraLinked(), isTimeStepLinked(), isResultColorControlled(), isLegendDefinitionsControlled(), isPropertyFilterOveridden(), updateOverrides(), updateDuplicatedPropertyFilters(), updateCameraLink(), updateTimeStepLink(), updateResultColorsControl(), updateLegendDefinitions() and applyCellFilterCollectionByUserChoice(). All now store ownerViewLinker() in a local and null-check it before use, matching the pattern already used in isCellFiltersControlled().
…nd missing singletons Guard all public entry points against null plot/data-source/curve/ensemble arguments, null-check RiaGuiApplication::instance() and RimMainPlotCollection::current() before dereferencing, and skip null curves/data sources when collecting existing data sources.
dir^up becomes a zero vector when the view direction and up vector are parallel, producing an invalid zero-normal plane. Plane::distanceToOrigin() then asserts/aborts (CVF_ASSERT, always active). Fall back to an arbitrary axis to compute a valid right vector in this degenerate case.
- Fix out-of-bounds vector access in calculateFormation when an index derived from curve data falls outside formationNamesVector, including when the value is invalid (NaN/inf) or negative. - Initialize m_stimPlanModel to nullptr in the constructor; it was previously left uninitialized until setStimPlanModel() was called. - Guard all entry points that dereference m_stimPlanModel directly (extractCurveData, calculateLayers, findCurveAndComputeLayeredAverage, findCurveAndComputeTopOfLayer, calculateStressWithGradients, calculateTemperature, calculateFacies, calculateFormation) and return a safe default/log an error when it is null. - Bounds-check findValueAtTopOfLayer/findValueAtBottomOfLayer against both the layer index and the underlying values vector, returning quiet_NaN instead of out-of-bounds access or a thrown exception from vector::at(). - Guard computeAverageByLayer against out-of-range indices and empty layers (division by zero), returning 0.0 for empty layers instead of NaN/inf.
kriben
requested changes
Oct 5, 2026
kriben
left a comment
kriben
left a comment
Collaborator
There was a problem hiding this comment.
Mostly good stuff! Some of the commit messages talk about being cherry-picked from some other commit, but I could not make sense of it. So probably better to remove the comment in the commit messages?
kriben
reviewed
Oct 5, 2026
kriben
left a comment
kriben
left a comment
Collaborator
There was a problem hiding this comment.
Additional findings from a full branch review.
magnesj
force-pushed
the
multiple-crash-fixes
branch
from
October 5, 2026 06:53
460879c to
abbfc3c
Compare
- Camera::fitView/computeFitViewEyePosition: derive one corrected, non-parallel up vector and use it consistently for right, upNorm, planeTop and the final setFromLookAt(), instead of only repairing the right vector. Previously the degenerate (parallel dir/up) case still produced a singular view matrix because up itself was never corrected. Added a regression test exercising fitView() end-to-end (not just computeFitViewEyePosition()) that checks the resulting direction/up/right form a valid orthonormal basis. - RimViewController::updateCameraLink/updateTimeStepLink: guard against ownerViewLinker()->masterView() being null (RimViewLinker::setMasterView(nullptr) is a supported state), not only against a null view linker. - RicHistogramPlotTools: use RiaGuiApplication::isRunning() before calling RiaGuiApplication::instance(), since instance() itself asserts when there is no running GUI application. - RimDockWindowController::updateViewerWidget: create the view widget before the dock widget, so a failed view-widget creation does not leave a dock widget allocated without a dock manager (which deleteDockWidget() cannot clean up). - RimStimPlanModelCalculator::calculateFormation: avoid continue via a small lambda, and reject double values outside the range of int before the static_cast to avoid undefined behavior.
Member
Author
|
Addressed all review feedback in 54291c6:
Re commit messages: checked the current 8 commits on this branch and none mention cherry-picking from other commits/repos, so I believe that's already fine. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Batch of crash fixes found via automated crash-report stacktrace triage, built
and combined on one branch for convenience. See #14848 for the consolidated
stacktraces.
This PR combines the following, each of which also has its own standalone PR
open (listed for reference — merging either this PR or the individual ones
makes the other redundant):
YsinRigStimPlanFractureDefinition::minY/maxY— Fix crash in RigStimPlanFractureDefinition::minY/maxY on empty Ys #14838RimDockWindowControllerwhen view widget creation fails — Fix crash in RimDockWindowController when view widget creation fails #14842RimViewController::masterViewwhen detached from view linker — Fix crash in RimViewController::masterView when detached from view linker #14843Camera::computeFitViewEyePositionwith parallel dir/up — Fix crash in Camera::computeFitViewEyePosition with parallel dir/up #14845RimStimPlanModelCalculator::calculateFormation— Fix out-of-bounds vector access in RimStimPlanModelCalculator::calculateFormation #14846Plus additional defensive hardening not tied to a specific reported crash:
RicHistogramPlotTools: null-guard every public entry point (plot, datasource, curve, collection, ensemble arguments) and
RiaGuiApplication/RimMainPlotCollectionsingleton lookups.RimStimPlanModelCalculator: initializem_stimPlanModeltonullptrin theconstructor (was previously left uninitialized), null-guard every entry point
that dereferences it, bounds-check
findValueAtTopOfLayer/findValueAtBottomOfLayer, and guardcomputeAverageByLayeragainstdivision by zero on empty layers.
Verification
ResInsightandResInsight-teststargets (RelWithDebInfo, existingbuild folder/config).
Fixes #14848.