Skip to content

Clarify thread-safety of SharedFlow methods in docs #2399

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
Nov 18, 2020

Conversation

elizarov
Copy link
Contributor

  • Override MutableSharedFlow.emit to attach a more appropriate docs than the one inherited from FlowCollector.
  • Clarify thread-safety of all the MutableSharedFlow & MutableState "mutating" methods.

The latter is needed, because Flows, in general, are sequential, but shared flows provide all the necessarily synchronization themselves, so, to avoid confusion it makes sense to additionally mention thread-safety of shared flows in all the relevant mutating functions.

* Override MutableSharedFlow.emit to attach a more appropriate docs than the one inherited from FlowCollector.
* Clarify thread-safety of all the MutableSharedFlow & MutableState "mutating" methods.

The latter is needed, because Flows, in general, are sequential, but shared flows provide all the necessarily synchronization themselves, so, to avoid confusion it makes sense to additionally mention thread-safety of shared flows in all the relevant mutating functions.
@elizarov elizarov requested a review from qwwdfsad November 18, 2020 08:14
Copy link
Contributor

@qwwdfsad qwwdfsad left a comment

Choose a reason for hiding this comment

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

A bit weird, because we already have an explicit mention of this behaviour in class documentation

* All methods of shared flow are **thread-safe** and can be safely invoked from concurrent coroutines without

E.g. we do not expect to have each method of ConcurrentHashMap to be documented as thread-safe, do we?

I don't really mind, it just a bit excessive

@Tolriq
Copy link

Tolriq commented Nov 18, 2020

@qwwdfsad HashMap method does not explicitly have methods with docs that says it's not thread safe. Here IDE on emit says in big not thread safe making people doubt.

@elizarov
Copy link
Contributor Author

elizarov commented Nov 18, 2020

I believe that excessiveness is warranted here, because a regular Flow is sequential and, in general, a FlowCollector.emit cannot be used concurrently. SharedFlow is an exception and stressing this difference is important (not everyone will read long SharedFlow doc in full).

@elizarov elizarov merged commit 31a8df0 into develop Nov 18, 2020
@elizarov elizarov deleted the shared-flow-safety-docs branch November 18, 2020 09:25
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.

3 participants