-
Notifications
You must be signed in to change notification settings - Fork 758
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
JSPI Fuzzing: Interleave executions #7226
Changes from 1 commit
93c4196
e220432
211d585
9610d85
ade1ebe
bf5d537
8b25475
5514a2c
96df913
02b6bfd
824908c
ef26967
1ef833a
70046a7
ba381d4
adc74f2
5fbb4d5
5af6120
9e68a3b
ed01751
d01a605
0f3b1dd
bfb4d38
c053377
8d1a559
94d9d01
d806a95
6931fd5
7b00d01
5bf3756
0d7668a
e7c55e3
b793595
a44418f
d479e99
14816ee
48e9392
686d39b
ea31e3c
09badf2
686403f
b7abf06
13a4a4d
3c246b9
663a667
110069b
3a57dad
d570dcc
639c964
f4d4311
9cca44c
3bfdbe5
15fa8fe
2daccd2
1dd5feb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -441,8 +441,9 @@ function hashCombine(seed, value) { | |
} | ||
|
||
// When we are changing up the order, in JSPI we can also leave some | ||
// Promises unresolved until later, which lets us interleave them. | ||
if (JSPI && ordering !== undefined) { | ||
// Promises unresolved until later, which lets us interleave them. Note we | ||
// never defer a task more than once (which would be pointless). | ||
if (JSPI && ordering !== undefined && !task.deferred) { | ||
if (result && typeof result == 'object' && | ||
typeof result.then === 'function') { | ||
// Hash with -1 here, just to get something different than the hashing a | ||
|
@@ -453,11 +454,12 @@ function hashCombine(seed, value) { | |
result = /* await */ result; | ||
} else { | ||
// Defer it for later. Reuse the existing task for simplicity. | ||
console.log(`(defer ${task.name})`); | ||
console.log(`(jspi: defer ${task.name})`); | ||
task.func = /* async */ () => { | ||
console.log(`(finish ${task.name})`); | ||
return /* await */ result | ||
console.log(`(jspi: finish ${task.name})`); | ||
return /* await */ result; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this await be in a try/catch as well? If the suspended Wasm ends up throwing an error, that error would propagate out here IIUC. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe the try-catch on 468 is enough? This code ends up run in that try-catch, so yes, an exception would propagate, but just to that try-catch, where it is caught and handled. (Note that we can't really add any handling for it here - all we could do is log and rethrow?) |
||
}; | ||
task.deferred = true; | ||
tasks.push(task); | ||
continue; | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this is all in
if (JSPI)
, do theasync
andawait
need to be comments?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My (perhaps overly paranoid) approach was that when JSPI is off, we don't want the JS VM to see any async stuff, which could affect codegen. Like maybe they lay out the stack or basic blocks differently in async functions?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would hope these isolated awaits/asyncs would not affect other code. Anway, I'm fine with leaving them since it's consistent with all the other uses.