Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Silence errors about invalid deferred callables during tests #79992

Closed

Conversation

RandomShaper
Copy link
Member

Addresses the complaint in #78987 (comment):

So we also get a lot of "ERROR: Trying to execute a deferred call/notification/set on a previously freed instance. Consider using queue_free() instead of free()." from running tests (you can see the CI run for this very PR). How should we avoid that?

NOTE: I'm not very fond of having to tweak the build system to let the TESTS_ENABLED macro reach the message queue implementation. Maybe the queue should expose some way of disabling the message. Or maybe even the error should only be printed when the debugger is attached so both the editor and tests are already out of the equation? Suggestions welcome.

@YuriSizov
Copy link
Contributor

Can we actually identify the issues and fix them instead of trying to get progressively more clever about silencing the warning? What is the cause of the issue in general, and how do we find the source of it in tests, in PM, in editor?

@RandomShaper
Copy link
Member Author

In most (all?) cases it's CanvasItem::_redraw_callback about Controls already deleted. At first sight, I can't think of an easy fix.

@reduz
Copy link
Member

reduz commented Jul 29, 2023

I am very unhappy with this pattern repeating

  1. Code is broken in a PR because of a misunderstanding of how it works
  2. Instead of understanding why it breaks, try to band-aid it.

IMO this should be closed and the original PR reverted and a lot of care needs to be taken when dealing with core.
Explanation here: #78987 (comment)

@RandomShaper RandomShaper deleted the mute_excessive_mq_errors branch July 31, 2023 09:40
@AThousandShips AThousandShips removed this from the 4.2 milestone Jul 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants