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

[CP][stable channel] Ensure start/stop file watching requests are run on the dart thread (Additional Change) #46298

Closed
a-siva opened this issue Jun 8, 2021 · 3 comments
Assignees
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. cherry-pick-approved Label for approved cherrypick request cherry-pick-review Issue that need cherry pick triage to approve

Comments

@a-siva
Copy link
Contributor

a-siva commented Jun 8, 2021

commit(s) to merge: 05e5427

merge instructions: clean merge

What is the issue:

A cherry pick for the original problem was done in #46249, but another issue was noted during the cherry pick review which is addressed by this change.

numerous user reports of the analysis server stopping responding to IDE requests; this is due to the isolate running the analysis server deadlocking when unsubscribing from file watching events

What is the fix:

At present start/stop requests are scheduled on RunLoop thread. This results in deadlocks since same RunLoop thread might be busy with blocking writes of file watching events, not giving a chance for Dart to read previously-written events. Reading would unblock writer.

This fix moves start/stop requests to run on Dart thread instead.

Why cherrypick:
lots of users reports; likely manifesting in a few separate issues
Risk:

The change has rolled into flutter without any issues.

Link to original issue(s):

#45996
/cc @kevmoo @mit-mit @whesse @athomas @vsmenon @franklinyow @a-siva @aam @DanTup

@a-siva a-siva added the cherry-pick-review Issue that need cherry pick triage to approve label Jun 8, 2021
@a-siva a-siva added the area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. label Jun 8, 2021
@franklinyow franklinyow added the cherry-pick-approved Label for approved cherrypick request label Jun 8, 2021
@franklinyow
Copy link
Contributor

Description:
Additional fix for analysis server stop responding to IDE requests , see #45996 for detail

Approved in SCRUM
CC: @kf6gpe

@athomas
Copy link
Member

athomas commented Jun 9, 2021

This doesn't merge cleanly into stable:

$ git diff
diff --cc runtime/bin/main_options.h
index 658cea3fd3d,4fab09cc36d..00000000000
--- a/runtime/bin/main_options.h
+++ b/runtime/bin/main_options.h
@@@ -48,7 -48,9 +48,13 @@@ namespace bin 
    V(suppress_core_dump, suppress_core_dump)                                    \
    V(enable_service_port_fallback, enable_service_port_fallback)                \
    V(disable_dart_dev, disable_dart_dev)                                        \
++<<<<<<< HEAD
 +  V(long_ssl_cert_evaluation, long_ssl_cert_evaluation)
++=======
+   V(long_ssl_cert_evaluation, long_ssl_cert_evaluation)                        \
+   V(bypass_trusting_system_roots, bypass_trusting_system_roots)                \
+   V(delayed_filewatch_callback, delayed_filewatch_callback)
++>>>>>>> 05e5427800f ([io/mac] Ensure FSEventsWatcher::Node is deleted synchronously with Callback that uses it.)
  
  // Boolean flags that have a short form.
  #define SHORT_BOOL_OPTIONS_LIST(V)

@athomas
Copy link
Member

athomas commented Jun 9, 2021

Merged to stable in 516eb3b (2.13.3).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. cherry-pick-approved Label for approved cherrypick request cherry-pick-review Issue that need cherry pick triage to approve
Projects
None yet
Development

No branches or pull requests

3 participants