Skip to content
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

Jump to bottom

spawnFunction needs to use a script snapshot of the parent isolate when starting #6610

Closed
a-siva opened this issue Nov 7, 2012 · 7 comments
Closed

spawnFunction needs to use a script snapshot of the parent isolate when starting #6610

a-siva opened this issue Nov 7, 2012 · 7 comments
Assignees
@liamappelbe
Labels
area-vm Use area-vm for VM related issues, including code coverage, FFI, and the AOT and JIT backends. type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)

Comments

@a-siva
Copy link
Contributor

a-siva commented Nov 7, 2012

Currently when isolate.spawnFunction is called it reloads
the same script that the parent isolate loaded but we don't
have a way of ensuring that the same versions of the script
got loaded in the child isolate.

Instead of loading scripts the spawnFunction should use
a script snapshot of the parent isolate.
@iposva-google
Copy link
Contributor

Added Area-VM label.

@iposva-google
Copy link
Contributor

Removed Priority-Medium label.
Added Priority-Unassigned label.

@iposva-google
Copy link
Contributor

Issue #19228 has been merged into this issue.

cc @dgrove.
cc @munificent.

@dgrove
Copy link
Contributor

dgrove commented Jun 5, 2014

Nathan and I believe that this is slowing down the spawning of transformer isolates in pub. Can this be addressed in the relatively near future?

@nex3
Copy link
Member

nex3 commented Apr 7, 2015

Marked this as blocking #23105.

@a-siva a-siva added Type-Defect area-vm Use area-vm for VM related issues, including code coverage, FFI, and the AOT and JIT backends. labels Apr 7, 2015
@a-siva a-siva self-assigned this Apr 7, 2015
This was referenced Jun 3, 2015
Closed
Closed
@a-siva a-siva mentioned this issue Jun 3, 2015
Closed
@DartBot DartBot mentioned this issue Jun 4, 2015
Closed
@nex3 nex3 mentioned this issue Dec 10, 2015
Closed
@nex3 nex3 mentioned this issue Jan 14, 2016
Closed
@kevmoo kevmoo added type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) and removed priority-unassigned labels Feb 29, 2016
@nex3 nex3 mentioned this issue Nov 13, 2018
Closed
@a-siva a-siva assigned liamappelbe and unassigned a-siva Nov 28, 2018
@a-siva
Copy link
Contributor Author

a-siva commented Nov 29, 2018

When this issue was first created we did not have the 'hot reload' feature, I think it is worth discussing the implications of hot-reloading and using the 'Isolate.spawn' API to spawn child isolates

  • If we spawn child isolates using the Isolate.spawn API and then hot reload the parent isolate or a child isolate we could potentially have isolates running different versions of the app and any messages passed consisting non primitive types could result in weird behavior
  • if we spawn a child isolate after a hot reload should it use the newly modified app script or the original one?

Getting this right in all situations would require having some kind of check pointing and getting all the isolates in the group reloaded with the new modified source. I don't believe it is worth spending engineering cycles trying to implement this corner case.

Here is what I propose

  • if we spawn child isolates and then hot reload an isolate in the group then only primitive values (null, num, bool, double, String), instances of SendPort, and lists and maps whose elements are any of these will be allowed in the messages to that isolate
  • if a spawn is done after a hot reload is done we will spawn the child isolate using the original app script an informational message could be produced recommending a restart in this case. Again in this case only the restricted set of messages would be allowed.

@a-siva a-siva mentioned this issue Nov 30, 2018
Closed
@a-siva
Copy link
Contributor Author

a-siva commented Nov 30, 2018

Opening a new issue #35302 for the hot reload issue per Liam's comment.

@mraleph mraleph mentioned this issue Dec 1, 2018
Closed
2 tasks
dart-bot pushed a commit that referenced this issue Dec 4, 2018
@liamappelbe
Load isolate from parent's kernel in Isolate.spawn calls.
e143a52 
Bug: #6610
Change-Id: Icd6e1c5d6d4b64b611fc58333c051a8a9a50679d
Reviewed-on: https://dart-review.googlesource.com/c/85724
Commit-Queue: Liam Appelbe <liama@google.com>
Reviewed-by: Vyacheslav Egorov <vegorov@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>
dart-bot pushed a commit that referenced this issue Dec 4, 2018
@liamappelbe
Revert "Load isolate from parent's kernel in Isolate.spawn calls."
63fd8f6 
This reverts commit e143a52.

Reason for revert: Broke all the reload-kernel bots.

Original change's description:
> Load isolate from parent's kernel in Isolate.spawn calls.
> 
> Bug: #6610
> Change-Id: Icd6e1c5d6d4b64b611fc58333c051a8a9a50679d
> Reviewed-on: https://dart-review.googlesource.com/c/85724
> Commit-Queue: Liam Appelbe <liama@google.com>
> Reviewed-by: Vyacheslav Egorov <vegorov@google.com>
> Reviewed-by: Ryan Macnak <rmacnak@google.com>

TBR=vegorov@google.com,rmacnak@google.com,asiva@google.com,liama@google.com

Change-Id: Ie9c305256da9b6478153b99502d0c63b0f43a3e6
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: #6610
Reviewed-on: https://dart-review.googlesource.com/c/86082
Reviewed-by: Liam Appelbe <liama@google.com>
Commit-Queue: Liam Appelbe <liama@google.com>
@a-siva a-siva mentioned this issue Apr 16, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@liamappelbe liamappelbe

Labels
area-vm Use area-vm for VM related issues, including code coverage, FFI, and the AOT and JIT backends. type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@nex3 @kevmoo @dgrove @iposva-google @liamappelbe @a-siva