Unconditionally setup tasks for resource destructors - #14158
Conversation
This commit updates the translation of resource destructors in the component model to unconditionally setup a sync task as the spec specifies. This resolves an issue where context slots were leaking across boundaries when a component destroyed its own resource. While here this updates the translation to use the inline fast path that is present in sync-to-sync adapters by juggling some code to make it sharable by the two locations.
fitzgen
left a comment
There was a problem hiding this comment.
r=me with nitpick below addressed
Also, do you want to have a disas test as well?
There was a problem hiding this comment.
Test coverage of the symmetric trapping cases? Probably worth exercising trapping before and after forcing the lazy thread.
There was a problem hiding this comment.
Could you expand on the trapping cases you're thinking of? The original intention for these tests was testing context.{get,set} in the context of resource destructors as opposed to trapping (where before it was just bad context set)
There was a problem hiding this comment.
I was thinking when the resource destructor itself traps, and then we do something else with the store to check that the thread context is cleaned up correctly for those new calls.
There was a problem hiding this comment.
Oh we don't clean up anything on traps, not only here but in tons of places. That generally works out though because once an instance traps it can't ever be used again, so there can't be a test anyway for reusing a component after a trap.
This commit updates the translation of resource destructors in the component model to unconditionally setup a sync task as the spec specifies. This resolves an issue where context slots were leaking across boundaries when a component destroyed its own resource.
While here this updates the translation to use the inline fast path that is present in sync-to-sync adapters by juggling some code to make it sharable by the two locations.