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

Make PortalsPubSub.publish non-blocking #25

Merged
merged 1 commit into from May 18, 2022
Merged

Conversation

Steven0351
Copy link
Member

fix: The publish function was blocking callers who publish until all subscribers were published to. In practice this is unlikely to be noticeable, but if there are many subscribers it could potentially block the caller for an inordinate amount of time.

chore: Add test helpers for Combine and update tests to use them.

chore: Remove NSLog from objective-c tests.

subscribers were published to. In practice this is unlikely to be
noticeable, but if there are many subscribers it could potentially block
the caller for an inordinate amount of time.
chore: Add test helpers for Combine and update tests to use them.
chore: Remove NSLog from objective-c tests.
@@ -11,7 +11,7 @@ import Capacitor

/// An interface that enables marshalling data to and from a ``Portal`` over an event bus
public enum PortalsPubSub {
private static let queue = DispatchQueue(label: "io.ionic.portals.pubsub", attributes: .concurrent)
private static let queue = DispatchQueue(label: "io.ionic.portals.pubsub")
Copy link
Member Author

Choose a reason for hiding this comment

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

This is now a serial queue because we want to ensure messages are delivered in order

@@ -69,7 +69,7 @@ public enum PortalsPubSub {
/// - topic: The topic to unsubscribe from
/// - subscriptionRef: The subscriptionRef provided during subscription
public static func unsubscribe(from topic: String, subscriptionRef: Int) {
queue.async(flags: .barrier) {
queue.async {
Copy link
Member Author

Choose a reason for hiding this comment

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

The barrier flag is no longer needed because we're using a serial queue now.

import XCTest

extension Publisher {
func expectOutput(
Copy link
Contributor

Choose a reason for hiding this comment

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

I love this

@Steven0351 Steven0351 merged commit 9e76637 into main May 18, 2022
@Steven0351 Steven0351 deleted the to-sync-or-async branch May 18, 2022 16:02
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.

None yet

3 participants