BugFix: Agent ID's were not static during host functions. #1270
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.
Fixes two bugs found by @ptheywood (reported via Slack, no GitHub issue), and introduces 3 new tests that covers the former issues.
Bug 1
If agents were created during a host function, and then accessed via a DeviceAgentVector in the same host function (layer). The internal transfer from host agent creation storage to DeviceAgentVector would see their IDs regenerated. This bug was also present if performed across multiple init functions, as host agent creation is only performed at the end of the init "layer".
Resolution 1
DeviceAgentVector_impl::_insert()
was always assigning IDs, the check for ID already being set was commented out as assumed redundant(?). So it was uncommented, which resolved the failing test case.Bug 2
In the same edge-case as Bug 1, only the agent's ID variable would be sync'd to device.
Resolution 2
Inside
DeviceAgentVector_impl::_insert()
, the change notifications for all variables were only set at the very end of the method, however ifunbound_buffer.empty()
then it would exit before this was reached. So I moved the change notification up, also removed the specific ID change notification as this should be covered with all variables.The full test suite still passes (Windows/Debug/SEATBELTS=ON)