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

Queuing messages before attach can lead to out-of-order publishing #926

Closed
tcard opened this issue Dec 5, 2019 · 0 comments · Fixed by #929
Closed

Queuing messages before attach can lead to out-of-order publishing #926

tcard opened this issue Dec 5, 2019 · 0 comments · Fixed by #929
Milestone

Comments

@tcard
Copy link
Contributor

tcard commented Dec 5, 2019

As expected, before we get the CONNECTED protocol message, published messages get enqueued. When CONNECTED arrives, all pending messages are supposed to be published right away.

But we have a bug: a publish can happen after the CONNECTED arrives and before the pending messages are publishes and that new message will be published right at that moment (since we're CONNECTED), but before the pending messages are published.

2019-12-05 17:10:39.770991+0100 xctest[55987:2631276] DEBUG: (ARTRealtimeChannel.m:577) R:0x7fa3fe95bcf0 C:0x7fa3fe96ce90 (test-3-testing) protocol message 0x7fa3fe970b20 has been queued and merged in an existing message
2019-12-05 17:10:39.771279+0100 xctest[55987:2631276] DEBUG: (ARTRealtimeChannel.m:577) R:0x7fa3fe95bcf0 C:0x7fa3fe96ce90 (test-3-testing) protocol message 0x7fa3fce12c70 has been queued and merged in an existing message
2019-12-05 17:10:39.771599+0100 xctest[55987:2631276] DEBUG: (ARTRealtimeChannel.m:577) R:0x7fa3fe95bcf0 C:0x7fa3fe96ce90 (test-3-testing) protocol message 0x7fa3fce13940 has been queued and merged in an existing message
2019-12-05 17:10:39.771886+0100 xctest[55987:2631276] DEBUG: (ARTRealtimeChannel.m:577) R:0x7fa3fe95bcf0 C:0x7fa3fe96ce90 (test-3-testing) protocol message 0x7fa3fce143f0 has been queued and merged in an existing message
2019-12-05 17:10:39.772236+0100 xctest[55987:2631276] DEBUG: (ARTRealtimeChannel.m:577) R:0x7fa3fe95bcf0 C:0x7fa3fe96ce90 (test-3-testing) protocol message 0x7fa3fe968d70 has been queued and merged in an existing message

...

2019-12-05 17:10:39.779660+0100 xctest[55987:2631518] DEBUG: R:0x7fa3fe95bcf0 realtime is transitioning from 1 - Connecting to 2 - Connected

Now look how the message with name 41 sneaks in:

2019-12-05 17:10:39.781121+0100 xctest[55987:2631276] DEBUG: RS:0x7fa3fe95a160 ARTJsonLikeEncoder<msgpack> encoding '{
    action = 15;
    channel = "test-3-testing";
    messages =     (
                {
            connectionId = "z62Z13c-EJ";
            name = 41;
        }
    );
    msgSerial = 0;
}'; got: {length = 86, bytes = 0x84a96d73 67536572 69616c00 a86d6573 ... a6616374 696f6e0f }

And only later the big message with the enqueued messages is sent:

2019-12-05 17:10:39.798636+0100 xctest[55987:2631518] VERBOSE: RS:0x7fa3fe95a160 ARTJsonLikeEncoder<msgpack>: protocolMessageToDictionary {
    action = 15;
    channel = "test-3-testing";
    messages =     (
                {
            connectionId = "z62Z13c-EJ";
            name = 1;
        },
                {
            connectionId = "z62Z13c-EJ";
            name = 2;
        },
                {
            connectionId = "z62Z13c-EJ";
            name = 3;
        },
                {
            connectionId = "z62Z13c-EJ";
            name = 4;
        },
...

tcard added a commit that referenced this issue Dec 8, 2019
This was somehow left behind as part of the whole #620 effort.

Fixes #926.
@tcard tcard added this to the 1.1.13 milestone Dec 8, 2019
@tcard tcard closed this as completed in #929 Dec 9, 2019
tcard added a commit that referenced this issue Dec 9, 2019
* Make ARTRealtimeChannel.publish thread-safe.

This was somehow left behind as part of the whole #620 effort.

Fixes #926.

* ARTRealtimeChannel.publish: dispatch_sync instead of async.

Tests rely on this to observe the effects right after the method is
called.
maratal pushed a commit that referenced this issue Jul 19, 2023
Client lib spec: Define "upgrade" unambiguously
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

1 participant