Some deferred observers get destroyed during updates #2409
Merged
+38
−2
Conversation
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
The test gives a read access violation. The problem is that some elements in deferredObservers_ can be destroyed while ObservableSettings::enableUpdates() is running. Later in the call deferredObserver->update() is called on the already destructed element and it gives a read access violation. The example here uses the zero coupon inflation swap helper to demonstrate the issue. The zciis_ member is one of the deferred observers. When ObservableSettings::enableUpdates() is running ZeroCouponInflationSwapHelper::initializeDates() is called because the helper is notified and updateDates_ is true. zciis_ is re-assigned and the original pointee is destroyed. But the original pointee that has been destroyed is still in the deferredObservers_ set and update() gets called on it leading to the read access violation.
Add a bool runningDeferredUpdates_ that is set to true when deferredObservers_ are being processed in ObservableSettings::enableUpdates(). We keep a set invalidDeferredObservers_ that holds elements of deferredObservers_ that are destroyed while ObservableSettings::enableUpdates() is running. We can then skip calling update on them in ObservableSettings::enableUpdates().
francisduffy
added a commit
to francisduffy/QuantLib
that referenced
this pull request
Dec 24, 2025
Contributor
Author
|
I added a slightly alternative approach in the commit francisduffy@62d968a. It changes the container for |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 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.
I noticed an issue with deferred updates in that elements of the
deferredObservers_set can become null during theObservableSettings::enableUpdates()call. The problem is that at the start of that method hereQuantLib/ql/patterns/observable.cpp
Line 30 in ecefd25
updatesDeferred_is set to false. Then, there is the loop overdeferredObservers_callingdeferredObserver->update()on each. It is during this loop, that the call toupdate()for adeferredObserver"earlier" in thedeferredObservers_set leads to the destruction of adeferredObserver"later" in thedeferredObservers_set. The destructor of the laterdeferredObserveris called andObserver::~Observer()andobservable->unregisterObserver(this)are calledQuantLib/ql/patterns/observable.hpp
Line 220 in ecefd25
The issue is that
updatesDeferred_is at this pointfalseand hence the line hereQuantLib/ql/patterns/observable.hpp
Line 199 in ecefd25
deferredObserverthat is destructed is left in thedeferredObservers_set.I added a test in commit c2b5cc8 to demonstrate the issue. In commit ad90c6f, I added a fix. The fix is based on the proposal by @pcaspers here. Let me know what you think. I can make amendments if you think the fix can be improved.