Closed Bug 1137330 Opened 5 years ago Closed 5 years ago

Sharing empty file over bluetooth fails

Categories

(Firefox OS Graveyard :: Bluetooth, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(firefox39 fixed)

RESOLVED FIXED
2.2 S8 (20mar)
Tracking Status
firefox39 --- fixed

People

(Reporter: gerard-majax, Assigned: ben.tian)

Details

Attachments

(1 file, 1 obsolete file)

Reproduced at least on Shinano and Aries.

STR:
 0. Shake device to produce log
 1. Tap notification, share to bluetooth (requires gaia from one or two days ago at least)

Expected:
 All files are shared

Actual:
 Sharing gets stuck at some point, leaving a pending transfer that never ends.

After poking around I found it was the sharing of "dev-log-events.log" logshake-produced file that was hanging like this. This file is 0 byte because the /dev/log/events is empty.

Sharing all of the other logshake-produced file over bluetooth works, so I suspect that the fact that the file is 0 byte has a role here.
I do reproduce with different symptoms on my Flame rebuilt just of now:
 - sharing a non-0 byte file fails constantly (and I don't know why, this used to work, and I flashed boot and system partitions) with a failure error notification
 - sharing a 0-byte file hangs and I never get an error
Alexandre, can you provide the 0-byte file you use? Also how did you share log from gallery/music/video apps? Please also let us know if you installed some 3rd party app for sharing.

(In reply to Alexandre LISSY :gerard-majax from comment #1)
> I do reproduce with different symptoms on my Flame rebuilt just of now:
>  - sharing a non-0 byte file fails constantly (and I don't know why, this
> used to work, and I flashed boot and system partitions) with a failure error
> notification
>  - sharing a 0-byte file hangs and I never get an error
Flags: needinfo?(lissyx+mozillians)
(In reply to Ben Tian [:btian] from comment #3)
> Alexandre, can you provide the 0-byte file you use? Also how did you share
> log from gallery/music/video apps? Please also let us know if you installed
> some 3rd party app for sharing.

You can't create a 0 byte file ? The issue is not related to using a specific application for sharing: it happened with logshake, and with several "file explorer" apps from Marketplace. STR are already documented in comment 0.
Flags: needinfo?(lissyx+mozillians)
(In reply to Alexandre LISSY :gerard-majax from comment #4)
> You can't create a 0 byte file ? The issue is not related to using a
> specific application for sharing: it happened with logshake, and with
> several "file explorer" apps from Marketplace. STR are already documented in
> comment 0.

Alexandre, did you accept/reject the 0-byte file transfer on receiver? Instead of stuck transfer as comment 0, "File could not be sent" pops on notification tray when I accept/reject on receiver. Note I transfer the 0-byte file with app "Explorer" [1] on Flame m-c build (with bug 1143563 fix).

[1] https://marketplace.firefox.com/app/explorer
Flags: needinfo?(lissyx+mozillians)
I did accept all the file transfer, of course. The error you are seeing is what I documented in comment 2, when trying to reproduce on my Flame. Two weeks ago.
Flags: needinfo?(lissyx+mozillians)
(In reply to Ben Tian [:btian] from comment #5)
> (In reply to Alexandre LISSY :gerard-majax from comment #4)
> > You can't create a 0 byte file ? The issue is not related to using a
> > specific application for sharing: it happened with logshake, and with
> > several "file explorer" apps from Marketplace. STR are already documented in
> > comment 0.
> 
> Alexandre, did you accept/reject the 0-byte file transfer on receiver?
> Instead of stuck transfer as comment 0, "File could not be sent" pops on
> notification tray when I accept/reject on receiver. Note I transfer the
> 0-byte file with app "Explorer" [1] on Flame m-c build (with bug 1143563
> fix).
> 

Why are you talking about bug 1143563 ? Does this one can impact what I am seeing ? I doubt, it's a regression from bug 1141442 which landed after my initial report.
(In reply to Alexandre LISSY :gerard-majax from comment #7)
> Why are you talking about bug 1143563 ? Does this one can impact what I am
> seeing ? I doubt, it's a regression from bug 1141442 which landed after my
> initial report.

I tested on the latest m-c and meet following error without bug 1143563 fix, so I applied the fix for further testing. Just note be clear and I don't think this bug is its regression.

GeckoConsole: [JavaScript Error: "TypeError: this.manager.isOPPProfileConnected is not a function" {file: "app://system.gaiamobile.org/js/bluetooth_transfer_icon.js" line: 11}]
The patch sets opcode as "PutFinal" instead of "Put" for file header packet if file size is zero.

In usual transfers, gecko bluetooth sends 1 "Put" packet for file header, multiple "Put" packets for file body, and then one "PutFinal" packet to indicate completion. However for 0-byte files, the first packet for file header already completes and should be "PutFinal" one instead.
Assignee: wiwang → btian
Attachment #8577974 - Flags: review?(shuang)
Comment on attachment 8577974 [details] [diff] [review]
Patch 1 (v1): Handle 0-byte file for bluetooth OPP

Review of attachment 8577974 [details] [diff] [review]:
-----------------------------------------------------------------

Good catch!

::: dom/bluetooth/bluedroid/BluetoothOppManager.cpp
@@ +329,5 @@
>      // non STATE_MOUNTED. Then |OnSocketDisconnect| would also be called to
>      // handle remaining files during send(|DiscardBlobsToSend|) or
>      // receive(|DeleteReceivedFile|).
> +    BT_LOGR("Volume state is not STATE_MOUNTED. "
> +            "Abort any ongoing OPP connection.");

It's not related to this bug. For coding style fix, please open an another bug.
Attachment #8577974 - Flags: review?(shuang) → review+
(In reply to Ben Tian [:btian] from comment #9)
> Created attachment 8577974 [details] [diff] [review]
> Patch 1 (v1): Handle 0-byte file for bluetooth OPP
> 
> The patch sets opcode as "PutFinal" instead of "Put" for file header packet
> if file size is zero.
> 
> In usual transfers, gecko bluetooth sends 1 "Put" packet for file header,
> multiple "Put" packets for file body, and then one "PutFinal" packet to
> indicate completion. However for 0-byte files, the first packet for file
> header already completes and should be "PutFinal" one instead.

Please add these explanation in your patch description.
Revise per reviewer's comment. Also apply fix to bluez & bluedroid folders of bluetooth and bluetooth2.
Attachment #8577974 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/c26e59068ac1
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → 2.2 S8 (20mar)
You need to log in before you can comment on or make changes to this bug.