Skip to content

Add StartImmediate to MailboxProcessor #14931

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 8 commits into from
Aug 17, 2023

Conversation

bmitc
Copy link
Contributor

@bmitc bmitc commented Mar 20, 2023

F# language suggestion: fsharp/fslang-suggestions#997
Original PR by @xloggr: #11515


This PR implements both a static and instance StartImmediate method to MailboxProcessor to start a MailboxProcessor on the current thread. Normally, Start will start the MailboxProcessor on the thread pool, but the StartImmediate method ensures that the MailboxProcessor starts on the same thread as the one that calls it. Effectively, MailboxProcessor.Start and MailboxProcessor.StartImmediate map to Async.Start and Async.StartImmediate, respectively.

This is useful for situations in which you want to force the MailboxProcessor to start on a given thread or even run on a single thread throughout its lifetime. For example, a MailboxProcessor may be desired to wrap some unmanaged state, such as a window or some communication session that is not thread safe, and this is not currently possible with MailboxProcessor.Start. To start a MailboxProcessor on a specific thread, simply create that thread and then call MailboxProcessor.StartImmediate on it.

Thanks goes to @xloggr for the original suggestion and PR! This PR basically just takes that PR and adds a test and the documentation for StartImmediate.

@bmitc bmitc force-pushed the add-start-immediate-to-mailbox-processor branch from ddfc233 to 673d6fa Compare August 1, 2023 03:17
Copy link
Contributor Author

@bmitc bmitc left a comment

Choose a reason for hiding this comment

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

I've left a self-review asking some questions that I have regarding general development, a typo in an existing test message, and primarily regarding the idea and implementation of this feature.

@bmitc bmitc marked this pull request as ready for review August 1, 2023 07:46
@bmitc bmitc requested a review from a team as a code owner August 1, 2023 07:46
@psfinaki psfinaki mentioned this pull request Aug 2, 2023
@0101 0101 marked this pull request as draft August 14, 2023 17:10
@0101
Copy link
Contributor

0101 commented Aug 14, 2023

Converting to draft for now. Let us know when you feel like it's ready for review.

@bmitc
Copy link
Contributor Author

bmitc commented Aug 14, 2023

It is ready for review, which is why I converted it from draft to ready for review a week or two ago. I have been waiting for review and was planning on pinging on it today.

The feature is implemented, has a test, and I added the signatures and documentation in the mailbox.fsi file. There's nothing more to do, that I know of, besides having it reviewed.

@bmitc bmitc marked this pull request as ready for review August 14, 2023 17:29
@psfinaki
Copy link
Member

@bmitc alright, I will take a look on this tomorrow - thanks!

@bmitc
Copy link
Contributor Author

bmitc commented Aug 15, 2023

Sounds good! Thank you!

Copy link
Member

@psfinaki psfinaki left a comment

Choose a reason for hiding this comment

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

I am up for taking this in. I understand that the usage is limited but the API is clean, the docs and tests are fine as well.

Thanks for this!

@psfinaki psfinaki merged commit 0dfd880 into dotnet:main Aug 17, 2023
@bmitc
Copy link
Contributor Author

bmitc commented Aug 17, 2023

Thanks all for the help, discussion, review, and merge! 🎉

@bmitc bmitc deleted the add-start-immediate-to-mailbox-processor branch August 17, 2023 19:57
jwosty pushed a commit to jwosty/fsharp that referenced this pull request Aug 18, 2023
* Add `StartImmediate` static and instance members to `MailboxProcessor`

* Remove unused `Microsoft.FSharp.Collections` declaration

* Add test for `MailboxProcessor.StartImmediate`

* Rename test to corrected, intended name and "fix" test

* Update baseline tests

* Slight update to documentation

* Fix baseline tests file

* Add additional commentary to test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

5 participants