Prevent segfault when unloading from spillable container #75299
Merged
Conversation
This file contains 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
github-actions
bot
added
<Bugfix>
github-actions
bot
added
astyled
Prevents segfault from accessing an invalidated `item_location` when
unloading from a container that itself is inside a spillable container.
The previous segfault could be reproduced by having an inventory like:
```
backpack >
steel_pan >
bottle_plastic_small > antacid tablet (1)
bottle_plastic_small > antacid tablet (4)
```
And then unloading the single pill bottle caused a segfault. What
happens is that part of unloading will trigger the steelpan to spill
its contents. Part of spilling the contents makes the `item_location`
invalidated, as previously documented in `::handle_contents_changed` in
`character.h`.
This commit instead makes sure that we do not use `item_location` or the
item after we have called `contents_change_handler::handle_by`. From the
example above, the steelpan will still spill its contents, but the
segfault can no longer be reproduced. This commmit also adds a
user-visible message when spilling non-liquid items.
inogenous
force-pushed
the
prevent-segfault-unloading-spillable-container
branch
from
07f3682 to
8573bd7
Compare
github-actions
bot
added
the
BasicBuildPassed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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
Bugfixes "Prevent segfault when unloading from spillable container"
Purpose of change
Prevents segfault from accessing an invalidated
item_locationwhen unloading from a container that itself is inside a spillable container. Fixes #75156 .Describe the solution
The previous segfault could be reproduced by having an inventory like:
And then unloading the single pill bottle caused a segfault. What happens is that part of unloading will trigger the steelpan to spill its contents. Part of spilling the contents makes the
item_locationinvalidated, as previously documented in::handle_contents_changedincharacter.h.Cataclysm-DDA/src/character.h
Lines 2129 to 2130 in 67026ae
This change instead makes sure that we do not use
item_locationor the item after we have calledcontents_change_handler::handle_by. From the example above, the steelpan will still spill its contents, but the segfault can no longer be reproduced. This change also adds a user-visible message when spilling non-liquid items to make it obvious to the player what happened (there was no message at all before).Describe alternatives you've considered
Testing
Can no longer reproduce the segfault in #75156 with this change.
Additional context
It's odd that we were able to put the antacid-pill-bottles in the steelpan in the first place, since it's a spillable container, and that should not be possible in a backpack. But that's for a different issue.