Skip to content

Conversation

@jdm
Copy link
Member

@jdm jdm commented Nov 27, 2025

This implements the solution from #26488 that prevents implementing Drop manually for DOM interfaces, which is very easy to write in a way that can cause memory safety errors during JS runtime teardown. Since we have a bunch of pre-existing implementations that we are working on removing, this PR adds an opt-out mechanism that those interfaces use.

Testing: Compile-time code generation; not possible to test.
Fixes: part of #26488

@servo-highfive servo-highfive added the S-awaiting-review There is new code that needs to be reviewed. label Nov 27, 2025
Copy link
Member

@mrobinson mrobinson left a comment

Choose a reason for hiding this comment

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

Interesting. It seems that this is mainly an issue for WebGPU, WebGL, and a single cookie DOM object.

@servo-highfive servo-highfive removed the S-awaiting-review There is new code that needs to be reviewed. label Nov 27, 2025
@jdm
Copy link
Member Author

jdm commented Nov 27, 2025

Yeah, @willypuzzle has already fixed a bunch of other interfaces, but it mainly affects interfaces that gave some kind of registration/unregistration pattern.

…entations.

Signed-off-by: Josh Matthews <josh@joshmatthews.net>
@jdm jdm force-pushed the dom-drop-ratchet branch from bd76d23 to 8c1f6a0 Compare November 27, 2025 19:47
@jdm jdm requested a review from gterzian as a code owner November 27, 2025 19:47
@servo-highfive servo-highfive added the S-awaiting-review There is new code that needs to be reviewed. label Nov 27, 2025
@jdm jdm enabled auto-merge November 27, 2025 19:48
@jdm jdm added this pull request to the merge queue Nov 27, 2025
@servo-highfive servo-highfive added the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Nov 27, 2025
Merged via the queue into servo:main with commit c2bfb1c Nov 27, 2025
32 checks passed
@jdm jdm deleted the dom-drop-ratchet branch November 27, 2025 20:34
@servo-highfive servo-highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Nov 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-awaiting-review There is new code that needs to be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants