Bug 91485 - OBEX does not wait for final packet to be sent to client
Summary: OBEX does not wait for final packet to be sent to client
Status: RESOLVED FIXED
Alias: None
Product: SyncEvolution
Classification: Unclassified
Component: SyncEvolution (show other bugs)
Version: unspecified
Hardware: Other Linux (All)
: medium normal
Assignee: Patrick Ohly
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-07-28 10:08 UTC by James Laird-Wah
Modified: 2017-02-23 13:43 UTC (History)
2 users (show)

See Also:
i915 platform:
i915 features:


Attachments
wait before cancelling during shutdown (4.38 KB, patch)
2015-08-14 12:29 UTC, Patrick Ohly
Details | Splinter Review

Description James Laird-Wah 2015-07-28 10:08:14 UTC
Hello,
I'm using SyncEvolution 1.5.1 with libsynthesis  libsynthesis_3.4.0.47+syncevolution-1-5-1 on a Gentoo machine, with OpenObex 1.7.1.
I'm attempting to sync calendar+todo, contacts and memo with a Nokia 208 (Series 40) running their latest firmware.
Sync appears to work (SyncEvolution completes successfully), but the device always requests a slow sync and gives a zero LastAnchor. In the on-device sync log, it claims that the sync failed (with the helpful message "Status: Data transfer not possible. Check data connection.")

Investigating with Wireshark shows that the last message (acknowledgement) from SyncEvolution does not go over the OBEX link before the disconnection.
The following patch ensures the last packet goes out, and sync succeeds at the device end; incremental syncing starts to work. I'm pretty sure this isn't the correct way to handle this case, but I can't quite wrap my head around the session state machine, sorry!

I can't sanitise the session log but am happy to provide it privately if necessary.

--- a/src/syncevo/ObexTransportAgent.cpp
+++ b/src/syncevo/ObexTransportAgent.cpp
@@ -325,6 +325,7 @@ void ObexTransportAgent::send(const char *data, size_t len) {
     m_sock = sockObj;
     m_channel = channel;
     m_obexEvent = obexEventSource;
+    OBEX_HandleInput(m_handle, 1000);
 }
 
 /*
Comment 1 Patrick Ohly 2015-08-14 12:25:05 UTC
The developer who wrote that code is no longer around; I'm looking at this in detail for the first time myself, so I also have some problems trying to understand how the connection needs to be closed correctly.

The patch you sent just seems to ensure that the connection gets serviced longer than usual, so the real problem must be some missing code that waits for the proper termination of the connection.

In Wireshark, I see this at the end (copied one-by-one with context menu->Copy->Summary (Text) - if there's a better way I'm all ears):

103	2.569266000	localhost ()	Nokia_1e:ad:30 (Nokia N97 mini)	L2CAP	17	Sent Disconnection Request (SCID: 0x0041, DCID: 0x0041)
104	2.573606000	controller	host	HCI_EVT	8	Rcvd Number of Completed Packets
105	2.575667000	Nokia_1e:ad:30 (Nokia N97 mini)	localhost ()	L2CAP	17	Rcvd Disconnection Response (SCID: 0x0041, DCID: 0x0041)
106	4.579482000	host	controller	HCI_CMD	7	Sent Disconnect
107	4.580612000	controller	host	HCI_EVT	7	Rcvd Command Status (Disconnect)
108	4.666611000	controller	host	HCI_EVT	7	Rcvd Disconnect Complete

This looks normal to me. What's your corresponding trace?

When you say "the last message (acknowledgement) from SyncEvolution does not go over the OBEX link" are you talking about the SyncML message or some lower-level OBEX/RFCOMM/whatever message?
Comment 2 Patrick Ohly 2015-08-14 12:29:35 UTC
Created attachment 117682 [details] [review]
wait before cancelling during shutdown

After adding debug messages and looking at the code more closely, I can imagine that the (normal) shutdown() call might cancel the transfer of the last SyncML message if it happens too quickly after the send().

Without the additional wait(true) in shutdown(), the debug output shows a OBEX_EV_LINKERR. That error goes away when waiting for the OBEX_EV_REQDONE for the OBEX_CMD_PUT.

However, the phones that I have tested with so far never had the problem and even when there is such a OBEX_EV_LINKERR, the N97 mini that I was testing with now didn't seem to care.

James, does this patch fix the problem for you?
Comment 3 Patrick Ohly 2016-09-21 08:41:31 UTC
I'm taking the patch into SyncEvolution 1.5.2 release candidates. Some more testing with that (once available as binaries) would be good.
Comment 4 Patrick Ohly 2017-02-23 13:43:43 UTC
Fixed as part of 1.5.2.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.