fix (callbacks): Refcount mess in mvAddCallback() and mvRunCallback() #2580
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.
name: Pull Request
about: Create a pull request to help us improve
title: Refcount mess in mvAddCallback() and mvRunCallback()
assignees: ''
Closes #2036
Description:
This fix looks remarkably similar to PR #2282 by @bzczb, who deserves recognition for finding all the hidden
gemsquirks and overall for great job on fixing the issue (trust me, this rabbit hole was deep). However, in fact this PR is the result of "parallel evolution" of my own fix, and therefore bears some differences, too. One of them is how callbacks in the queue are handled when their "parent"mvAppItemgets deleted - currently both fixes have some visible side effects in this case. I'm going to add yet another fix on top of this (as a separate PR but still the same release) and try to keepdelete_itembehavior backward compatible for most scenarios/users.This PR also includes some changes that got mixed with it over time and I decided to bring them along rather than waste time trying to separate things:
on_closecall to popup windows - this fixes on_close has no effect for popup windows #2372.item_visiblehandler now passes the ID of the widget inapp_data, like other handlers do.destroy_contextnow works way better and waits for all callbacks to complete before shutting down everything - fixes Resize handlers might be running while DPG context is being destroyed #2020.Also, the PR fixes some other issues that are not directly related to reference counting but are related to the callback mechanism:
set_frame_callback- this fixes set_frame_callback is racy and can lose callbacks #2269.Concerning Areas:
After merging this PR,
delete_itemwill behave a bit differently: if a callback for the item being deleted is running (including the case where you calldpg.delete_item(sender)from the callback), the item will be kept alive until the callback completes. All subsequent callbacks waiting in the queue will be thrown away, but the current callback will work as if the item never got deleted. Well, you won't be able to actually access the item, but it will prevent creation of another one with the same tag/UUID 😅.I'm going to add yet another PR to reduce some unexpected side effects of this change in behavior.