Skip to content

Conversation

@ksuprynowicz
Copy link
Member

Thanks to Gouldron for the backtrace!

@ksuprynowicz ksuprynowicz added bug Something isn't working needs CR This pull request needs to be code reviewed needs QA This pull request needs to be tested labels Dec 27, 2025
@JulianGro
Copy link
Member

What does this fix? XMLHttpRequest works fine on the latest release on my machine™. Is this a crash when closing a script that is still waiting on a response to its XMLHttpRequest?

@ksuprynowicz
Copy link
Member Author

It fixes a crash on newest libnode. On release we still have the old libnode.

@ksuprynowicz ksuprynowicz added QA approved This pull request has been successfully tested and removed needs QA This pull request needs to be tested labels Jan 3, 2026
_v8Object.Reset();
QObject* qobject = _object;
if(qobject) qobject->deleteLater();
if(qobject) delete qobject;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add an assertion to check this is running on the right thread?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! I added an assert and it uncovered underlying issue with ScriptableResource which I fixed too.

_qobjectWrapperMap.clear();
_qobjectWrapperMapV8.clear();
}
// Events need to be processed one more time:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe a comment stating this is for completing pending deleteLater calls?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed!

@ada-tv
Copy link
Collaborator

ada-tv commented Jan 3, 2026

Fixes a crash for me when disconnecting from ceon

@ada-tv ada-tv added CR approved This pull request has been successfully code reviewed and removed needs CR This pull request needs to be code reviewed labels Jan 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working CR approved This pull request has been successfully code reviewed QA approved This pull request has been successfully tested

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants