You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Follow-up to #14401, which introduced caf::PdmXmlObjectHandle::initAfterInsert() and migrated the 18 sites that already called resolveReferencesRecursively() + initAfterReadRecursively() as a pair. The single-call sites were deliberately left alone, since adding the missing half is a behavior change that has to be judged per site.
This issue lists the remaining sites, grouped by what the review has to decide.
A. Init only, object already inserted (2 sites)
Both call initAfterReadRecursively() after the object has been added to its parent, so the pointer fields deserialized from the copy are never resolved. This is the same shape as the crash in #14372.
RicSummaryPlotEditorUi.cpp:580 — plot->addCurveNoUpdate( curveCopy ) at line 578, then init only. Note that setSummaryCaseY() is called manually at line 575, so adding resolve would overwrite the manual assignment with the reference string carried over from the source curve. This site needs the interleaving used in RicPasteSummaryCurveFeature, not a plain swap.
B. Resolve only, copy then insert (13 sites)
The references are resolved, but initAfterRead() overrides are not re-run after the copy. Each site needs a check of whether any object in the copied subtree has an initAfterRead() that does more than read fields.
RicDuplicateJobFeature.cpp:60 (already calls initAfterCopy() before insertion)
RicCreateRftPlotsFeature.cpp:111
RicAddStoredWellAllocationPlotFeature.cpp:70
RicAddStoredFlowCharacteristicsPlotFeature.cpp:71
RicPasteModeledWellPathFeature.cpp:154
RicCreateMultipleWellPathLaterals.cpp:153
RimGeoMechModels.cpp:99
RimUserDefinedCalculationCollection.cpp:93
RiaSummaryPlotTools.cpp:230 and RiaSummaryPlotTools.cpp:416
RimSummaryMultiPlot.cpp:1721
RicAppendSummaryPlotsForObjectsFeature.cpp:79 and :108 — like RicSummaryPlotEditorUi, manual case and ensemble assignment follows the resolve call, so the ordering is load bearing
RicHistogramPlotTools.cpp:197 — resolve is behind a resolveRefs parameter. Worth checking whether the caller that passes false is inserting a freshly constructed curve, in which case the parameter can be removed
C. Deserialize into an object already in the tree (2 sites)
A different pattern: an existing object reads XML into itself, so there is no insertion step. initAfterInsert() applies as far as the assert goes, but the question of whether initAfterRead() should re-run is separate.
For the sites where manual assignment follows resolve (RicSummaryPlotEditorUi, RicAppendSummaryPlotsForObjectsFeature), move initAfterInsert() to immediately after insertion and do the manual assignment afterwards, so the resolved value is the one that gets overwritten and not the other way around.
Consider a debug-only diagnostic in the framework that reports pointer fields still holding an unresolved reference string once an object becomes reachable from the document root. That would catch the whole class of bugs without requiring every future call site to be reviewed by hand.
Follow-up to #14401, which introduced
caf::PdmXmlObjectHandle::initAfterInsert()and migrated the 18 sites that already calledresolveReferencesRecursively()+initAfterReadRecursively()as a pair. The single-call sites were deliberately left alone, since adding the missing half is a behavior change that has to be judged per site.This issue lists the remaining sites, grouped by what the review has to decide.
A. Init only, object already inserted (2 sites)
Both call
initAfterReadRecursively()after the object has been added to its parent, so the pointer fields deserialized from the copy are never resolved. This is the same shape as the crash in #14372.RicNewStatisticsContourMapViewFeature.cpp:100—statisticsContourMap->addView( contourMapView )at line 96, then init only. ARimStatisticsContourMapViewcarries the same legend config pointer fields as the contour map view that crashed in Crash creating contour map from 3D view with ternary result and intersections #14372.RicSummaryPlotEditorUi.cpp:580—plot->addCurveNoUpdate( curveCopy )at line 578, then init only. Note thatsetSummaryCaseY()is called manually at line 575, so adding resolve would overwrite the manual assignment with the reference string carried over from the source curve. This site needs the interleaving used inRicPasteSummaryCurveFeature, not a plain swap.B. Resolve only, copy then insert (13 sites)
The references are resolved, but
initAfterRead()overrides are not re-run after the copy. Each site needs a check of whether any object in the copied subtree has aninitAfterRead()that does more than read fields.RicCopyIntersectionsToAllViewsInCaseFeature.cpp:123RicDuplicateJobFeature.cpp:60(already callsinitAfterCopy()before insertion)RicCreateRftPlotsFeature.cpp:111RicAddStoredWellAllocationPlotFeature.cpp:70RicAddStoredFlowCharacteristicsPlotFeature.cpp:71RicPasteModeledWellPathFeature.cpp:154RicCreateMultipleWellPathLaterals.cpp:153RimGeoMechModels.cpp:99RimUserDefinedCalculationCollection.cpp:93RiaSummaryPlotTools.cpp:230andRiaSummaryPlotTools.cpp:416RimSummaryMultiPlot.cpp:1721RicAppendSummaryPlotsForObjectsFeature.cpp:79and:108— likeRicSummaryPlotEditorUi, manual case and ensemble assignment follows the resolve call, so the ordering is load bearingRicHistogramPlotTools.cpp:197— resolve is behind aresolveRefsparameter. Worth checking whether the caller that passesfalseis inserting a freshly constructed curve, in which case the parameter can be removedC. Deserialize into an object already in the tree (2 sites)
A different pattern: an existing object reads XML into itself, so there is no insertion step.
initAfterInsert()applies as far as the assert goes, but the question of whetherinitAfterRead()should re-run is separate.RimRegularLegendConfig.cpp:1051(setUiValuesFromLegendConfig)RimDialogData.cpp:107(setExportCarfinDataFromString)D. Intentionally not migrated
Listed so the next reader does not have to re-derive it. Adding a short comment at each site would make that explicit.
RiaApplication.cpp:591-598,:932, and the VFP re-resolve at:696, which interleave withcreateDerivedObjectsRecursively()RiaApplication.cpp:1697andRicEditPreferencesFeature.cpp:81— document roots, not inserted into a parent fieldRicfCommandFileExecutor.cpp:94— command objects are not part of the project treecafPdmXmlObjectHandle.cpp:249and:277— framework internals, the object is not inserted yetRiaSummaryPlotTemplateTools.cpp:160-163,:214-217andRicPasteSummaryCurveFeature.cpp:52-58— custom interleaving between the two callsSuggested approach
RicNewStatisticsContourMapViewFeature, which is the closest match to the crash that motivated Introduce initAfterInsert() to combine resolveReferencesRecursively() and initAfterReadRecursively() #14401.RicSummaryPlotEditorUi,RicAppendSummaryPlotsForObjectsFeature), moveinitAfterInsert()to immediately after insertion and do the manual assignment afterwards, so the resolved value is the one that gets overwritten and not the other way around.