Fix getGlobalCurrentTaskMeta null handling#1378
Fix getGlobalCurrentTaskMeta null handling#1378wondenge wants to merge 1 commit intobytecodealliance:mainfrom
Conversation
vados-cosmonic
left a comment
There was a problem hiding this comment.
Thanks for the contribution @wondenge -- I'd like to get the behavioral test in here before this merges -- the generated-code-only check doesn't really do much to help IMO, and regardless it would be better to check in the regression test with the fix.
9ce5933 to
dfaf1dd
Compare
Hey @vados-cosmonic sorry for the delay, been swamped this week. Thanks for cleaning things up in the meantime. I'd still like to add a behavioral test to this PR before it merges if that works for you - a test component in |
|
Hey @wondenge I really appreciate it, please take your time! BTW, Jco 1.17.7 is on it's way out the door (this link should work soon) -- so feel free to give that a try as well. The first implementations of |
Description
_getGlobalCurrentTaskMetatreats a cleared task slot (null) as a valid meta object._clearCurrentTasksetsCURRENT_TASK_META[componentIdx] = null, but the getter only checks forundefined, sonullpasses through and{...null}returns{}truthy, but with notaskID. This causes_lowerImportBackwardsCompatto skip its auto-task-creation fallback and throwinvalid/missing async task meta.This shows up when an import fires between two export calls. Export trampolines for types that need allocation (strings, lists, etc.) call
reallocvia_utf8AllocateAndEncodebefore creating their task. If the component has imports wired into internal functions (allocator internals likedlmalloc::memalign), those imports go through_lowerImportBackwardsCompatin a window whereCURRENT_TASK_METAisnullfrom the previous export's cleanup but no new task exists yet.The fix adds a
nullcheck alongside the existingundefinedcheck in the intrinsic codegen. A codegen test is added toinitialization.jsthat transpiles thenon-wizered-initcomponent and verifies the generated_getGlobalCurrentTaskMetaincludes the null check. This is a string match on generated code rather than a behavioral test, a proper integration test would need a component with imports wired into allocator internals to actually trigger the crash, which is hard to set up generically. The codegen assertion is a pragmatic alternative that catches accidental removal of the null check. I'll file a follow-up issue for a behavioral test that exercises the actual failure path.