On So, 2010-03-07 at 01:56 +0100, Patrick Ohly wrote: > Anyway, it fails in one case when the item was split: > 1. client has a new big item > 2. client sends <Add> with <MoreData/> > 3. server buffers the item > 4. client sends second half > 5. server processes the complete item, removes the PIStored blob, > sends reply
5.5. admin data is stored with lastitemstatus=0, in contrast to the 201 status stored there when the item is not split across messages. > 6. reply never reaches client > 7. client and server suspend > 8. upon resume, client resends the second half > (<Item><Source><LocURI>2273</LocURI></Source> > <Meta><EMI>datapos=19241</EMI></Meta><Data>...) > 9. server attempts to parse the item, fails and returns 415 > > The logs don't tell me what the relevant difference is, so I'll have to > dive into the debugger. I found something and got it working with the following patch. Lukas, does this make sense? commit ce704f6f521e9153d801f47d750a9a45fcc6f507 Author: Patrick Ohly <[email protected]> Date: Mon Mar 15 16:16:32 2010 +0100 resume + pending item fix in SyncML server When an item that was split among messages was finally reassembled and processed by the server, the status sent to the client was not properly stored in the admin data. As a result of that, when the session got suspended directly after sending that status, the client would resume, send the last item chunk again, but then not get the previous status. Instead the server attempted to parse the incomplete item and failed. The root cause was that TSyncSession::process() was called recursively. The inner one for the incomplete command saved the status in fLastItemStatus. The outer one then later didn't have a status pointer, fell back to status 0 and overwrote the valid status. This patch solves this by suppressing that second updating of fLastItemStatus and all related fields if the item was handled by an inner process() call. diff --git a/src/sysync/synccommand.cpp b/src/sysync/synccommand.cpp index 1892b24..73560cc 100755 --- a/src/sysync/synccommand.cpp +++ b/src/sysync/synccommand.cpp @@ -2270,6 +2270,7 @@ bool TSyncOpCommand::execute(void) localstatus sta; TSyncOpCommand *incompleteCmdP; bool queueforlater,processitem; + bool nostatus; SmlItemListPtr_t tobequeueditems=NULL; SYSYNC_TRY { @@ -2292,6 +2293,7 @@ bool TSyncOpCommand::execute(void) while (*itemnodePP) { queueforlater=false; // do no queue by default processitem=true; // process by default + nostatus=false; // set to true if someone else is responsible for updating fLastItemStatus thisitemnode = *itemnodePP; // no result nor status so far statusCmdP=NULL; @@ -2531,7 +2533,10 @@ bool TSyncOpCommand::execute(void) // - first remove global link to it to avoid recursion fSessionP->fIncompleteDataCommandP=NULL; // - execute now (and pass ownership) - // issues appropriate statuses + // issues appropriate statuses, so we don't need to deal with it; + // in fact, we must not touch fLastItemStatus because we don't + // know the status + nostatus=true; fSessionP->process(incompleteCmdP); // - this item is processed now, continue in loop if there are more items processitem=false; // do not process the item normally @@ -2628,7 +2633,7 @@ bool TSyncOpCommand::execute(void) // item processed // - remember status (final only) for possible suspend and resume sta= statusCmdP ? statusCmdP->getStatusCode() : 0; - if (sta!=213) { + if (!nostatus && sta!=213) { // final status received, save it for possible resend fDataStoreP->fLastItemStatus = sta; // but forget data stored at DS level -- Best Regards, Patrick Ohly The content of this message is my personal opinion only and although I am an employee of Intel, the statements I make here in no way represent Intel's position on the issue, nor am I authorized to speak on behalf of Intel on this matter. _______________________________________________ os-libsynthesis mailing list [email protected] http://lists.synthesis.ch/mailman/listinfo/os-libsynthesis
