servo
servo copied to clipboard
servo
Create send_request macro for WebGPURequest
- [x]
./mach build -ddoes not report any errors - [x]
./mach test-tidydoes not report any errors - [x] These changes fix #32488
- [x] These changes do not require tests because there is no behavior change
I think we can generalize this a bit more (adding optional custom error message using format syntax) and move it to ipc channel as similar pattern is everywhere not just in webgpu.
But firstly let's get some consensus on this. cc @mrobinson @wusyong @gterzian
I suspect that different modules will want to handle IPC failures in different ways. For instance in the case of WebGPU, it will likely need to shut down the task and return. On the other hand , the Constellation would probably want to restart the failed tasks.
I suspect that different modules will want to handle IPC failures in different ways. For instance in the case of WebGPU, it will likely need to shut down the task and return. On the other hand , the Constellation would probably want to restart the failed tasks.
In that cases when error is actually handled we would not use macro, but do manual handling. Macro would only serves as a shorter way to write send and warn on error:
if let Err(e) = sender.send(Msg) {
error!("Failed to send Msg due to error: {e}");
// nothing else here
}
Or are you suggesting that we should actually replace all such patterns with actual error handling not just logging, so why bother with making it easier.
Or are you suggesting that we should actually replace all such patterns with actual error handling not just logging, so why bother with making it easier.
Yes, I think that at some point failed IPC messages will need to be handled via proper error handling and recovery since a failed IPC message can mean that the task the message is delivering to has disappeared. Maybe in the simplest case that means shutting the entire task down, but in some cases (the constellation for example) more will need to be done.
Or are you suggesting that we should actually replace all such patterns with actual error handling not just logging, so why bother with making it easier.
Yes, I think that at some point failed IPC messages will need to be handled via proper error handling and recovery since a failed IPC message can mean that the task the message is delivering to has disappeared. Maybe in the simplest case that means shutting the entire task down, but in some cases (the constellation for example) more will need to be done.
should I do it part of this PR?
should I do it part of this PR?
If it's possible to write a macro to halt execution of the current function, trigger the shutdown of the WebGPU task cleaning up any resources, and then terminate the task -- that would be excellent, I think!
should I do it part of this PR?
If it's possible to write a macro to halt execution of the current function, trigger the shutdown of the WebGPU task cleaning up any resources, and then terminate the task -- that would be excellent, I think!
What is task in this context? Because if it's just "WebGPU work that was send", that would leave page in unexpected state. Maybe we can just soft-crash the tab (just replacing current page with tab crashed page, so no actual crashing, so cleanup would be done by GC). Given that ipc fails very rarely, this would be nice enough solution.
should I do it part of this PR?
If it's possible to write a macro to halt execution of the current function, trigger the shutdown of the WebGPU task cleaning up any resources, and then terminate the task -- that would be excellent, I think!
What is task in this context? Because if it's just "WebGPU work that was send", that would leave page in unexpected state. Maybe we can just soft-crash the tab (just replacing current page with tab crashed page, so no actual crashing, so cleanup would be done by GC). Given that ipc fails very rarely, this would be nice enough solution.
Does the WebGPU task keep any resources for ongoing Documents? If so, these will need to be cleaned up, even if the WebGPU task continues.
should I do it part of this PR?
If it's possible to write a macro to halt execution of the current function, trigger the shutdown of the WebGPU task cleaning up any resources, and then terminate the task -- that would be excellent, I think!
What is task in this context? Because if it's just "WebGPU work that was send", that would leave page in unexpected state. Maybe we can just soft-crash the tab (just replacing current page with tab crashed page, so no actual crashing, so cleanup would be done by GC). Given that ipc fails very rarely, this would be nice enough solution.
Does the WebGPU task keep any resources for ongoing Documents? If so, these will need to be cleaned up, even if the WebGPU task continues.
They are cleaned when Drop of associated dom objects is called (dropping GPUBuffer will send DropBuffer to wgpu thread), although if the thread is dead (that's probably the reason for IPC failure), everything should already be cleaned up on wgpu side.
They are cleaned when Drop of associated dom objects is called (dropping GPUBuffer will send
DropBufferto wgpu thread), although if the thread is dead (that's probably the reason for IPC failure), everything should already be cleaned up on wgpu side.
If the script process crashes, Drop implementations won't be called so what is the mechanism to ensure that resource are released on the WebGPU-side?
They are cleaned when Drop of associated dom objects is called (dropping GPUBuffer will send
DropBufferto wgpu thread), although if the thread is dead (that's probably the reason for IPC failure), everything should already be cleaned up on wgpu side.If the script process crashes,
Dropimplementations won't be called so what is the mechanism to ensure that resource are released on the WebGPU-side?
There isn't such mechanism (although wgpu internally does some cleanup beforehand, so the only thing that is kept alive is ID). Similar problem might also be present in WebGL.
I think we need to have more broad discussion how we will approach erroring and recovery in servo.
I think we need to have more broad discussion how we will approach erroring and recovery in servo.
Yes, I agree. In general, Servo needs to be make much more resilient to process crashes, task panics, and IPC failures. I guess my only concern here is that we shouldn't be hiding away places where we are going to need to add real error handling in the future.
I think we need to have more broad discussion how we will approach erroring and recovery in servo.
Yes, I agree. In general, Servo needs to be make much more resilient to process crashes, task panics, and IPC failures. I guess my only concern here is that we shouldn't be hiding away places where we are going to need to add real error handling in the future.
I think we need to have more broad discussion how we will approach erroring and recovery in servo.
Yes, I agree. In general, Servo needs to be make much more resilient to process crashes, task panics, and IPC failures. I guess my only concern here is that we shouldn't be hiding away places where we are going to need to add real error handling in the future.
I will close this PR, and I will check with @sagudev what we can do to add better error handling .