Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upScript Task should exit only when all of its associated pipelines have exited. #967
Comments
bors-servo
pushed a commit
that referenced
this issue
Sep 23, 2013
Fixes #967 and #965 This has been wrong for a long time. Previously, only the pipeline associated with the root frame evicted would be shut down. 1) It shouldn't necessarily be closed, because there could be references to it still in the navigation context, and 2) Presumably none of the children pipelines of the root frame were ever exiting. It's hard to test this right now because #965 covers up other pipeline exiting issues, but when that's fixed, a pathological case in which things would have broken down would be: 1) Load a page with an iframe that contains a link 2) Click the link 3) Press backspace to navigate back 4) Navigate to any new page, at which point the forward page would be evicted from the navigation context, and the outer frame's pipeline would be shut down improperly. 5) Press backspace, at which point there is no longer a pipeline for the old page, because it was shut down prematurely. Presumably this would cause a crash. I also changed the FrameTree function ```find_mut``` to ```find``` because find_mut implies it's doing something to cause mutability, but the mutability is caused by the type of object being iterated over, nothing else. Additionally, script was exiting completely when receiving an exit message. Instead, it needs to handle exit messages according to who sent it. It should only close the subframes of the frame whose pipeline sent the exit message. This is now fixed. Inexplicably, script was also closing the compositor upon receiving an exit message. This doesn't seem like it'd ever be the right thing to do. I've fixed that. I don't think anyone shuts down the compositor now. It seems like it should occur in servo.rc, but I haven't added it because I'm not positive.
bors-servo
pushed a commit
that referenced
this issue
Sep 23, 2013
Fixes #967 and #965 This has been wrong for a long time. Previously, only the pipeline associated with the root frame evicted would be shut down. 1) It shouldn't necessarily be closed, because there could be references to it still in the navigation context, and 2) Presumably none of the children pipelines of the root frame were ever exiting. It's hard to test this right now because #965 covers up other pipeline exiting issues, but when that's fixed, a pathological case in which things would have broken down would be: 1) Load a page with an iframe that contains a link 2) Click the link 3) Press backspace to navigate back 4) Navigate to any new page, at which point the forward page would be evicted from the navigation context, and the outer frame's pipeline would be shut down improperly. 5) Press backspace, at which point there is no longer a pipeline for the old page, because it was shut down prematurely. Presumably this would cause a crash. I also changed the FrameTree function ```find_mut``` to ```find``` because find_mut implies it's doing something to cause mutability, but the mutability is caused by the type of object being iterated over, nothing else. Additionally, script was exiting completely when receiving an exit message. Instead, it needs to handle exit messages according to who sent it. It should only close the subframes of the frame whose pipeline sent the exit message. This is now fixed. Inexplicably, script was also closing the compositor upon receiving an exit message. This doesn't seem like it'd ever be the right thing to do. I've fixed that. I don't think anyone shuts down the compositor now. It seems like it should occur in servo.rc, but I haven't added it because I'm not positive.
bors-servo
pushed a commit
that referenced
this issue
Sep 23, 2013
Fixes #967 and #965 This has been wrong for a long time. Previously, only the pipeline associated with the root frame evicted would be shut down. 1) It shouldn't necessarily be closed, because there could be references to it still in the navigation context, and 2) Presumably none of the children pipelines of the root frame were ever exiting. It's hard to test this right now because #965 covers up other pipeline exiting issues, but when that's fixed, a pathological case in which things would have broken down would be: 1) Load a page with an iframe that contains a link 2) Click the link 3) Press backspace to navigate back 4) Navigate to any new page, at which point the forward page would be evicted from the navigation context, and the outer frame's pipeline would be shut down improperly. 5) Press backspace, at which point there is no longer a pipeline for the old page, because it was shut down prematurely. Presumably this would cause a crash. I also changed the FrameTree function ```find_mut``` to ```find``` because find_mut implies it's doing something to cause mutability, but the mutability is caused by the type of object being iterated over, nothing else. Additionally, script was exiting completely when receiving an exit message. Instead, it needs to handle exit messages according to who sent it. It should only close the subframes of the frame whose pipeline sent the exit message. This is now fixed. Inexplicably, script was also closing the compositor upon receiving an exit message. This doesn't seem like it'd ever be the right thing to do. I've fixed that. I don't think anyone shuts down the compositor now. It seems like it should occur in servo.rc, but I haven't added it because I'm not positive.
bors-servo
pushed a commit
that referenced
this issue
Sep 24, 2013
Fixes #967 and #965 This has been wrong for a long time. Previously, only the pipeline associated with the root frame evicted would be shut down. 1) It shouldn't necessarily be closed, because there could be references to it still in the navigation context, and 2) Presumably none of the children pipelines of the root frame were ever exiting. It's hard to test this right now because #965 covers up other pipeline exiting issues, but when that's fixed, a pathological case in which things would have broken down would be: 1) Load a page with an iframe that contains a link 2) Click the link 3) Press backspace to navigate back 4) Navigate to any new page, at which point the forward page would be evicted from the navigation context, and the outer frame's pipeline would be shut down improperly. 5) Press backspace, at which point there is no longer a pipeline for the old page, because it was shut down prematurely. Presumably this would cause a crash. I also changed the FrameTree function ```find_mut``` to ```find``` because find_mut implies it's doing something to cause mutability, but the mutability is caused by the type of object being iterated over, nothing else. Additionally, script was exiting completely when receiving an exit message. Instead, it needs to handle exit messages according to who sent it. It should only close the subframes of the frame whose pipeline sent the exit message. This is now fixed. Inexplicably, script was also closing the compositor upon receiving an exit message. This doesn't seem like it'd ever be the right thing to do. I've fixed that. I don't think anyone shuts down the compositor now. It seems like it should occur in servo.rc, but I haven't added it because I'm not positive.
ChrisParis
pushed a commit
to ChrisParis/servo
that referenced
this issue
Sep 7, 2014
New test: sync request blocks async
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Right now,
pipeline.exit()tells its Script Task and Render Task to exit. The Script Task then shuts down all associated layout tasks and exits. Shutting down all layouts that are contained in child pages ofpipeline'slayout is correct. But the Script Task itself shouldn't exit until the last pipeline it's associated with shuts down, which won't happen until the root pipeline is shut down.Also, it looks like the script task is shutting down the compositor, which doesn't seem like something that should ever happen from the script task.