Skip to content

[SourceKit] Allow explicit cancellation of requests with a cancellation token #39494

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

Merged
merged 2 commits into from
Sep 30, 2021

Conversation

ahoppen
Copy link
Member

@ahoppen ahoppen commented Sep 28, 2021

The key changes here are

  • To keep track of cancellation tokens for all ScheduledConsumers in SwiftASTManager
  • Generate unique request handles for all incoming requests (create_request_handle ), use these request handles as cancellation tokens and return them from the sourcekitd_send_request methods
  • Implement cancellation with sourcekitd_cancel_request as the entry point and SwiftASTManager::cancelASTConsumer as the termination point

Everything else is just plumbing the cancellation token through the various abstraction layers.

rdar://83391505

@ahoppen ahoppen requested a review from benlangmuir September 28, 2021 18:45
@ahoppen
Copy link
Member Author

ahoppen commented Sep 28, 2021

@swift-ci Please smoke test

@ahoppen ahoppen force-pushed the pr/sourcekit-cancellation-tokens branch 2 times, most recently from 95c3d7d to 649f351 Compare September 29, 2021 07:37
@ahoppen
Copy link
Member Author

ahoppen commented Sep 29, 2021

Thanks for your feedback @benlangmuir.

I addressed the comments and also moved the "msg" to Internal-XPC.h. I’m still using xpc_dictionary_set_value to set the cancellation token because

I thought because xpc_object_t’s definition is typedef void * xpc_object_t, xpc_object_t would just be an opaque pointer (which is what we are using for the cancellation token as well). Or am I making a wrong assumption here?

…on token

The key changes here are
- To keep track of cancellation tokens for all `ScheduledConsumer`s in `SwiftASTManager`
- Generate unique request handles for all incoming requests (`create_request_handle `), use these request handles as cancellation tokens and return them from the `sourcekitd_send_request` methods
- Implement cancellation with `sourcekitd_cancel_request` as the entry point and `SwiftASTManager::cancelASTConsumer` as the termination point

Everything else is just plumbing the cancellation token through the various abstraction layers.

rdar://83391505
@ahoppen ahoppen force-pushed the pr/sourcekit-cancellation-tokens branch from 649f351 to fb26447 Compare September 30, 2021 09:45
@ahoppen
Copy link
Member Author

ahoppen commented Sep 30, 2021

@swift-ci Please smoke test

Copy link
Contributor

@benlangmuir benlangmuir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

// We need to wait a little bit after request scheduling and cancellation to make sure we are not cancelling the request before it got scheduled.

// RUN: not %sourcekitd-test -req=cursor -id=slow -async -pos=10:3 %s -- %s == \
// RUN: -shell -- sleep 1 == \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought this would need // REQUIRES: shell, but I guess the tests passed on all platforms 🤷

@ahoppen ahoppen merged commit b0678ca into swiftlang:main Sep 30, 2021
@ahoppen ahoppen deleted the pr/sourcekit-cancellation-tokens branch September 30, 2021 15:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants