Messages in MessageQueue are picked up by Clients
A message that arrived out of order and is pending in the message queue could be picked up by getIntroductionMessages().
Activity
mentioned in issue #295 (closed)
Now that I come to think of it, perhaps there's a simpler way: create the sender's first message, create an outgoing sync session, throw away the resulting byte array. Create the sender's second message, create an outgoing sync session, deliver the byte array to the recipient's incoming sync session. Use a mock clock to ensure the first message isn't retransmitted in the second session. Assert that the second message was received and the first wasn't, then check whether the client picks up the second message.
@akwizgran I've been trying that and it looks like the message is picked up:
INFO: Received Response in state AWAIT_RESPONSES from Introducee1 to Introducee2 with session ID -141693337 in group -2058689361. Moving on to state AWAIT_RESPONSE_2 INFO: Forwarding message to group -1262445591 INFO: Sending message with position 1 INFO: TEST: Sending message from introducer0 to introducee2 INFO: Generated ack: false INFO: Generated batch: true INFO: Sent batch INFO: Generated batch: false INFO: Received message with position 1, expecting 0 INFO: Message is out of order, adding to pending list Apr 21, 2016 12:26:20 PM org.briarproject.IntroductionIntegrationTest$IntroduceeListener eventOccurred MessageValidatedEvent INFO: TEST: Introducee2 received message in group -1262445591At this point, I am doing
introductionManager2#getIntroductionMessagesand only do not get the response in the output, because the session state can not be found (because it didn't initialize it, because there was no request). But the message should not even be considered.The
if (msg == null)does not help here, because the message is already returned byclientHelper#getMessageMetadataAsDictionary. I guess it shouldn't be returned by that call as long as the message is still in the queue, right?Since a
MessageValidatedEventis triggered, I assume that the message is validated already and thus metadata created for it. Is this a problem with the way I sync messages or theMessageQueueitself?Milestone changed to %Milestone B
-
This is causing another issue (without negative consequences visible to the user):
08-01 12:46:37.837 I/MessageQueueManagerImpl: Received message with position 30, expecting 27 08-01 12:46:37.837 I/MessageQueueManagerImpl: Message is out of order, adding to pending list 08-01 12:46:37.887 I/ContactListFragment: Message added, reloading 08-01 12:46:37.897 I/MessageQueueManagerImpl: Received message with position 29, expecting 27 08-01 12:46:37.897 I/MessageQueueManagerImpl: Message is out of order, adding to pending list 08-01 12:46:37.977 I/ContactListFragment: Message added, reloading 08-01 12:46:37.977 I/MessageQueueManagerImpl: Received message with position 28, expecting 27 08-01 12:46:37.977 I/MessageQueueManagerImpl: Message is out of order, adding to pending list 08-01 12:46:38.027 I/ContactListFragment: Message added, reloading 08-01 12:46:38.157 I/DuplexOutgoingSession: Generated offer: true 08-01 12:46:38.157 I/DuplexOutgoingSession: Sent offer 08-01 12:46:38.277 I/ContactListFragment: Loading message headers took 108 ms 08-01 12:46:38.417 I/ContactListFragment: Loading introduction messages took 142 ms 08-01 12:46:38.617 W/SharingManagerImpl: No session state found for message with session ID -1917569721 08-01 12:46:38.637 W/SharingManagerImpl: org.briarproject.api.FormatException org.briarproject.api.FormatException at org.briarproject.sharing.SharingManagerImpl.getSessionState(SharingManagerImpl.java:634) at org.briarproject.sharing.SharingManagerImpl.getInvitationMessages(SharingManagerImpl.java:354) at org.briarproject.android.contact.ContactListFragment.getMessages(ContactListFragment.java:376) at org.briarproject.android.contact.ContactListFragment.access$200(ContactListFragment.java:63) at org.briarproject.android.contact.ContactListFragment$4.run(ContactListFragment.java:280) at org.briarproject.android.controller.DbControllerImpl$1.run(DbControllerImpl.java:35) at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1076) at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:569) at java.lang.Thread.run(Thread.java:856)@akwizgran how are we going to resolve this issue? Dropping the message queue in favor of message dependencies?
-
@grote yes, I think that's the best solution. Protocols that currently use message queues will have to be modified to include a predecessor ID in each message except the first. That makes the session ID in those messages redundant, and it's probably not a good idea to have a session ID and predecessor ID that could be inconsistent. So I guess we replace the session ID with a predecessor ID in all messages except the first - what do you think?
-
So I guess we replace the session ID with a predecessor ID in all messages except the first - what do you think?
That sounds good to me. The only issue with this is that it will be even harder to look up the session state without a session ID. We can of course recursively go through the previous messages, but this won't be very efficient.
-
A mostly unrelated thought: message queues define the order of messages sent in each direction, but they don't define the order of messages sent in one direction with respect to messages sent in the other direction. If we switch to dependencies we'll have a lot more flexibility about which messages are ordered with respect to each other. For example, we can make a response depend on the message it's responding to. But we'll have to be careful that whenever messages are meant to be ordered with respect to each other, there's a direct or indirect dependency relationship between them. I guess it will probably help to think about the dependency graph created by the messages alongside thinking about the state machine.
-
I was thinking the delivery hook could store the session ID in each message's metadata. For the first message in the session, the hook would get the session ID from the message itself. For subsequent messages, it would look up the session ID from the dependency's metadata.
That could work although it would not be a session ID anymore, but more like an internal session state storage ID. From the perspective of how session states are done at the moment, it would just be the
MessageIdof the local message that holds the metadata for the local state. -
That could work although it would not be a session ID anymore, but more like an internal session state storage ID. From the perspective of how session states are done at the moment, it would just be the MessageIdof the local message that holds the metadata for the local state.
Actually we have a choice here. We can include an explicit session ID in the first message sent between each pair of parties, and store that ID in the metadata of each subsequent message. Then we locally map the session ID to the ID of a message whose metadata contains the local state. For the time being that mapping has to be done via a hack, because we can't store metadata under arbitrary IDs, but when #506 (closed) has been implemented we can store the local state directly under the session ID. If the protocol involves more than two parties, we use the same session ID for all the pairwise interactions in a given session. This option is pretty close to what we do now, except we don't need to include the session ID in every message, only the first message sent between each pair of parties.
Alternatively, we can get rid of the explicit session ID altogether. We locally pick some message where we'll store the local state. For the purposes of session lookup, it doesn't matter what message we pick. We store the local state in the metadata of that message, and the ID of that message in the metadata of each message in the session. The nice thing about this approach is that we don't need any hacks. The delivery hook just looks up the metadata for the new message's dependency, extracts the message ID where the local state is stored, copies it to the new message's metadata, and looks up the local state. When #506 (closed) has been implemented, this option gets even nicer: we no longer need to pick a message, we just store the local state under a locally generated random ID, which is stored in the metadata of each message in the session.
Milestone changed to %Milestone E
Milestone changed to %Milestone F
-
I've created #896 (closed) for rewriting the transport client. Not sure if we'll have time to do it in this milestone, so we might have to deprioritise this ticket (although I'd like to get it done if possible).
changed milestone to %Android 1.0
mentioned in merge request !758 (merged)
assigned to @grote
mentioned in commit 68dfcf4e
mentioned in commit a9b678df
closed via commit a9b678df
closed via commit 31b69577
closed via merge request !758 (merged)
mentioned in commit 31b69577