Skip to content

Conversation

bnoordhuis
Copy link
Contributor

Introduced in commit 61c8fe6 from last month that moved the callback into the job queue:

  1. It leaked fre->held_val when no job was enqueued

  2. It fumbled the reference count when enqueuing; JS_EnqueueJob already takes care of incrementing and decrementing it

Reverts commit 0a70623 from earlier today because that didn't turn out to be a complete fix.

Fixes: #648

Introduced in commit 61c8fe6 from last month that moved the callback into the job queue: 1. It leaked `fre->held_val` when no job was enqueued 2. It fumbled the reference count when enqueuing; JS_EnqueueJob already takes care of incrementing and decrementing it Reverts commit 0a70623 from earlier today because that didn't turn out to be a complete fix. Fixes: quickjs-ng#648
@bnoordhuis
Copy link
Contributor Author

bnoordhuis commented Nov 6, 2024

Grr, tests/test_builtin.js still asserts, even with this change. It definitely fixes a bug but it clearly doesn't fix all bugs.

No wait, it did fix it! I didn't apply it right to another branch.

@saghul
Copy link
Contributor

saghul commented Nov 7, 2024

Thanks Ben!

@bnoordhuis bnoordhuis merged commit 9c5c441 into quickjs-ng:master Nov 7, 2024
47 checks passed
@bnoordhuis bnoordhuis deleted the fix648-for-realz branch November 7, 2024 08:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants