Hi Joshua,
I had a closer look and was able to reproduce the problem… I did have tests
that did syncing within a single process / thread, but they buffered the
messages in an array and sent them later; I’d never tested a delegate where the
-send*Message: method directly calls the corresponding -handle*: method.
Anyway I put in a hack that should fix the problem, along with some test cases:
https://github.com/etoile/CoreObject/commit/7f61fa24bf789b481e0b5a31ba648f1d0a6584dd#diff-f11577008ff9cc5807d3061a632e51aeR124
On Sep 23, 2014, at 6:27 AM, Joshua Kifer <[email protected]> wrote:
> Eric,
>
> I appreciate your detailed analysis. I spent some time stepping through the
> code and found that in COSynchronizerClient, it was doing the opposite of
> what it seem it should when it receives a message with the same UUID as the
> last one it sent. After removing the negation, the infinite loop is
> interrupted.
>
> - (void) handleResponseMessage:
> (COSynchronizerResponseToClientForSentRevisionsMessage *)aMessage
> {
> // if (![aMessage.lastRevisionUUIDSentByClient isEqual: [self
> lastRevisionUUIDInTransitToServer]])
> if ([aMessage.lastRevisionUUIDSentByClient isEqual: [self
> lastRevisionUUIDInTransitToServer]])
> {
> return;
> }
>
> It seems possible that the infinite loop may only occur in a
> single-application scenario such as this one, and why it does not happen in
> the sample app, a single context in a thread will either sort itself out or
> keep spinning, but will not get caught up in a sort of race condition where
> client/server are trying to sort themselves out in the same thread.
I think the intent of that check is to ignore "stale"
COSynchronizerResponseToClientForSentRevisionsMessage messages from the server.
It would only do anything if a
COSynchronizerResponseToClientForSentRevisionsMessage was delivered out of
order.
Sorry, it’s not very clear :-(
>
> I am also concerned with the following code at the end of
> COSynchronizerServer::sendPushToClient
>
> if (currentlyHandlingLastSentRevision != nil
> && [currentlyRespondingToClient isEqualToString: clientID])
> {
> ETAssert(currentlyRespondingToClient != nil);
>
> COSynchronizerResponseToClientForSentRevisionsMessage * message
> = [[COSynchronizerResponseToClientForSentRevisionsMessage alloc] init];
> message.revisions = revs;
> message.lastRevisionUUIDSentByClient =
> currentlyHandlingLastSentRevision;
>
> [self.delegate sendResponseMessage: message toClient: clientID];
>
> currentlyHandlingLastSentRevision = nil;
> currentlyRespondingToClient = nil;
> }
> else
> {
> COSynchronizerPushedRevisionsToClientMessage *message =
> [[COSynchronizerPushedRevisionsToClientMessage alloc] init];
> message.revisions = revs;
> [self.delegate sendPushedRevisions: message toClients:
> @[clientID]];
> }
>
> In my trace, the message that is created here is one with a single revision
> (the one previously sent by the client), and the lastRevisionUUIDSentByClient
> as being the same as the revision in the message. It seems like if this
> state were to occur, it should not be sent to the client in the first place
> where it is then handled in the code fixed handleResponseMessage.
Yeah, I noticed this too as I was debugging today, it is ugly that the server
sends back the full revision it just received from the client. Sending the full
revision contents back to the client could be avoided, although the server does
need to send a receipt saying “I got this particular revision UUID”.
>
> I wonder if I am the first to attempt a synchronizer client/server within the
> same thread. I will clean up and put this code up on GitHub for you to
> examine, and perhaps contribute to the CoreObject samples at some point.
>
> Thanks for your help.
No problem!
Just a few other things I thought of based on your first email..
- The COSynchronizer classes can’t deal with uncommitted changes. I added an
exception to make this clearer, previously things would have just broken.
- I think you may need to use something like COSynchronizerMessageTransport
from the test suite (or just borrow that class), which buffers messages.
With your current synchronizer delegate setup, If you type some text into both
text boxes without committing, then press one of the commit buttons, I suspect
you’ll get the exception I mentioned above when the message is delivered to the
other text box’s synchronizer object.
- Just a warning, while there is some code for diff/automatic merge of
COAttributedString, it doesn’t work very well, and it just does a
character-by-character diff.
Cheers
Eric_______________________________________________
Etoile-discuss mailing list
[email protected]
https://mail.gna.org/listinfo/etoile-discuss
