Closed Bug 1006271 Opened 10 years ago Closed 10 years ago

[Tarako][Email][LMK] Email attachment size limit logic bug results in too-large attachments being attached than detached rather than never detached. Results in email app getting killed.

Categories

(Firefox OS Graveyard :: Gaia::E-Mail, defect)

Product:
Firefox OS Graveyard
Component:
Gaia::E-Mail
Platform:
ARM
Gonk (Firefox OS)
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(blocking-b2g:1.3T+, b2g-v1.3T fixed, b2g-v1.4 fixed, b2g-v2.0 fixed)

Status:
VERIFIED FIXED
Milestone:
2.0 S1 (9may)
Project Flags:
blocking-b2g 1.3T+
Tracking Flags:
Tracking Status
b2g-v1.3T --- fixed
b2g-v1.4 --- fixed
b2g-v2.0 --- fixed

People

(Reporter: bli, Assigned: asuth)

Details

(Whiteboard: [sprd308061])

Attachments

(4 files)

logcat
91.16 KB, text/x-log
Details
slog.tar.gz
563.09 KB, application/x-gzip
Details
instead of attaching the detaching, just do not attach in the first place
46 bytes, text/x-github-pull-request
mcav
: review+
praghunath
: approval-gaia-v1.4+
Details | Review
logcat-email-attach-pic
88.34 KB, text/x-log
Details
Attached file logcat — 
Environment:
---------------------------------------------------
Gaia      48372ff88c712468a77a6f14a97f9978ee56eade
Gecko     https://hg.mozilla.org/releases/mozilla-b2g28_v1_3t/rev/78015cc18d1f
BuildID   20140505173556
Version   28.1
ro.build.version.incremental=eng.cltbld.20140505.162017
ro.build.date=Mon May  5 16:20:25 EDT 2014


Gaia      c067cc717c4a3bb8b412f28f37d224eb84b639df
Gecko     https://hg.mozilla.org/releases/mozilla-b2g28_v1_3t/rev/8eebf8120dbd
BuildID   20140505164002
Version   28.1
ro.build.version.incremental=eng.cltbld.20140505.203623
ro.build.date=Mon May  5 20:36:32 EDT 2014



Steps to reproduce:
---------------------------------------------------
1. Launch Email

2. Create a new mail

3. Tap on the clip icon on the top right

4. Select a large file whose size is larger than the limited , such as 28M video
   --> There should be a tip saying the attachment is too large to send with this message

5. Add attachment again, and this time the attachment could be smaller or larger than the limited size.


Actual result:
-------------------------------
Sometimes, email gets killed after step 5.
And sometimes, need to repeat step 5 for several times, like 3 times.
Good find!

So, the attachment limit logic (bug 8718952) was written against the pre bug 871897 logic.  Under that implementation logic, attachments were lazily committed, so it was fine to add the attachment and then remove it again.  However, under bug 871897, we end up attaching the attachment and then removing it.  So we try and encode all 28M, then remove it again.  When I implemented bug 871897 I didn't worry about this situation since I would not have implemented the UI logic in that manner, but since I reviewed 871852 and wrote bug 871897, this is obviously on me.

It actually seems conceivable that we might survive the 28 meg attachment if no other apps became active.  However, the logs indicate that an attempt re-add the attachment a second time so we have the overhead of another app while we are in our peak memory usage scenario.

This is a trivial fix and should be fixed for trunk, so I am going to take this bug and fix it.  I'd suggest we also uplift the fix to v1.3T since it will be minor and avoid this very regrettable situation.


Triage summary: If someone tries to attach a giant attachment, we tell them "no", but then go through the motions of attaching it just to detach it again.  This is dumb and does not work out well for anyone.  We crash and produce a lot of I/O churn in the process of crashing.
Assignee: nobody → bugmail
Status: NEW → ASSIGNED
blocking-b2g: --- → 1.3T?
Attached file slog.tar.gz — 
slog include system and kernel log
I can reproduce this issue and have some findings as below:
1. LMK works normal as we expected.
2. LMK was triggered due Email use more memory even if the attachment exceed limited size.

kernel log when Email was killed by LMK:
05-06 14:45:08.817 <4>0[ 1213.966708] lowmem_shrink select 1793 (E-Mail), adj 2, size 15435, to kill
05-06 14:45:08.817 <4>0[ 1213.966789] lowmem_shrink send sigkill to 1793 (E-Mail), adj 2, size 15435
05-06 14:45:08.817 <4>0[ 1213.966847] b2g invoked lowmemorykiller: gfp_mask=0x213da, oom_adj=0, oom_score_adj=0
05-06 14:45:08.817 <4>0[ 1213.966906] Backtrace: 
05-06 14:45:08.817 <4>0[ 1213.966969] [<c4537ad8>] (dump_backtrace+0x0/0x110) from [<c4889d80>] (dump_stack+0x18/0x1c)
05-06 14:45:08.817 <4>0[ 1213.967035]  r7:c670bc18 r6:000213da r5:c64fd7a0 r4:c670a000
05-06 14:45:08.817 <4>0[ 1213.967118] [<c4889d68>] (dump_stack+0x0/0x1c) from [<c477966c>] (lowmem_shrink+0x480/0x554)
05-06 14:45:08.817 <4>0[ 1213.967193] [<c47791ec>] (lowmem_shrink+0x0/0x554) from [<c45a4944>] (shrink_slab+0x114/0x1c0)
05-06 14:45:08.817 <4>0[ 1213.967267] [<c45a4830>] (shrink_slab+0x0/0x1c0) from [<c45a54fc>] (try_to_free_pages+0x1e8/0x368)
05-06 14:45:08.817 <4>0[ 1213.967347] [<c45a5314>] (try_to_free_pages+0x0/0x368) from [<c459d214>] (__alloc_pages_nodemask+0x368/0x598)
05-06 14:45:08.817 <4>0[ 1213.967433] [<c459ceac>] (__alloc_pages_nodemask+0x0/0x598) from [<c459f0a8>] (__do_page_cache_readahead+0x124/0x260)
05-06 14:45:08.836 20] [<c459ef84>] (__do_page_cache_readahead+0x0/0x260) from [<c459f210>] (ra_submit+0x2c/0x34)
05-06 14:45:08.836 <4>0[ 1213.967599] [<c459f1e4>] (ra_submit+0x0/0x34) from [<c4596da0>] (filemap_fault+0x19c/0x480)
05-06 14:45:08.836 <4>0[ 1213.967676] [<c4596c04>] (filemap_fault+0x0/0x480) from [<c45adb1c>] (__do_fault+0x54/0x404)
05-06 14:45:08.836 <4>0[ 1213.967749] [<c45adac8>] (__do_fault+0x0/0x404) from [<c45aeafc>] (handle_pte_fault+0x27c/0x6cc)
05-06 14:45:08.836 <4>0[ 1213.967825] [<c45ae880>] (handle_pte_fault+0x0/0x6cc) from [<c45af4c0>] (handle_mm_fault+0xd0/0xe4)
05-06 14:45:08.836 <4>0[ 1213.967907] [<c45af3f0>] (handle_mm_fault+0x0/0xe4) from [<c453b808>] (do_page_fault+0xe8/0x28c)
05-06 14:45:08.836 <4>0[ 1213.967989] [<c453b720>] (do_page_fault+0x0/0x28c) from [<c4528288>] (do_DataAbort+0x3c/0xa0)
05-06 14:45:08.836 <4>0[ 1213.968074] [<c452824c>] (do_DataAbort+0x0/0xa0) from [<c4533dcc>] (ret_from_exception+0x0/0x10)
05-06 14:45:08.836 <4>0[ 1213.968142] Exception stack(0xc670bfb0 to 0xc670bff8)
05-06 14:45:08.836 <4>0[ 1213.968190] bfa0:                                     001e84d5 00000015 00000083 400090fc
05-06 14:45:08.836 <4>0[ 1213.968260] bfc0: b000b548 b000b548 41e7d52b 40009528 40009da8 41e7d52b 00100000 00000001
05-06 14:45:08.836 <4>0[ 1213.968328] bfe0: 5f9df944 4453c3c0 b000131b b000131e 40800030 ffffffff
05-06 14:45:08.836 <4>0[ 1213.968380]  r7:40009528 r6:41e7d52b r5:00000007 r4:0000040f
05-06 14:45:08.836 <4>0[ 1213.968442] Mem-info:
05-06 14:45:08.836 <4>0[ 1213.968466] Normal per-cpu:
05-06 14:45:08.836 <4>0[ 1213.968598] CPU    0: hi:   42, btch:   7 usd:  41
05-06 14:45:08.836 <4>0[ 1213.968651] active_anon:5338 inactive_anon:5357 isolated_anon:104
05-06 14:45:08.836 <4>0[ 1213.968659]  active_file:736 inactive_file:711 isolated_file:0
05-06 14:45:08.836 <4>0[ 1213.968667]  unevictable:83 dirty:1 writeback:9 unstable:0
05-06 14:45:08.836 <4>0[ 1213.968674]  free:321 slab_reclaimable:249 slab_unreclaimable:1112
05-06 14:45:08.836 <4>0[ 1213.968682]  mapped:2655 shmem:304 pagetables:414 bounce:0
05-06 14:45:08.836 <4>0[ 1213.968890] Normal free:1284kB min:1352kB low:2200kB high:2540kB active_anon:21352kB inactive_anon:21428kB active_file:2944kB inactive_file:2844kB unevictable:332kB isolated(anon):416kB isolated(file):0kB present:114408kB mlocked:0kB dirty:4kB writeback:36kB mapped:10620kB shmem:1216kB slab_reclaimable:996kB slab_unreclaimable:4448kB kernel_stack:2008kB pagetables:1656kB unstable:0kB bounce:0kB writeback_tmp:0kB pages_scanned:0 all_unreclaimable? no
05-06 14:45:08.836 <4>0[ 1213.969156] lowmem_reserve[]: 0 0 0
05-06 14:45:08.836 <4>0[ 1213.969200] Normal: 81*4kB 2*8kB 3*16kB 6*32kB 7*64kB 2*128kB 0*256kB 0*512kB 0*1024kB 0*2048kB 0*4096kB = 1284kB
05-06 14:45:08.836 <4>0[ 1213.969328] 4085 total pagecache pages
05-06 14:45:08.836 <4>0[ 1213.969361] 203 pages in swap cache
05-06 14:45:08.836 <4>0[ 1213.969395] Swap cache stats: add 164093, delete 163890, find 5606/77725
05-06 14:45:08.836 <4>0[ 1213.969446] Free swap  = 1588kB
05-06 14:45:08.836 <4>0[ 1213.969475] Total swap = 65532kB
05-06 14:45:08.836 <4>0[ 1213.971009] 28854 pages of RAM
05-06 14:45:08.836 <4>0[ 1213.971043] 1030 free pages
05-06 14:45:08.836 <4>0[ 1213.971070] 1707 reserved pages
05-06 14:45:08.836 <4>0[ 1213.971099] 996 slab pages
05-06 14:45:08.836 <4>0[ 1213.971124] 8028 pages shared
05-06 14:45:08.836 <4>0[ 1213.971152] 203 pages swap cached
05-06 14:45:08.836 <6>0[ 1213.971182] [ pid ]   uid  tgid total_vm      rss cpu oom_adj oom_score_adj name
05-06 14:45:08.836 <6>0[ 1213.971257] [   65]     0    65       78       22   0     -16          -941 ueventd
05-06 14:45:08.836 <6>0[ 1213.971324] [   80]  1000    80      207       38   0     -16          -941 servicemanager
05-06 14:45:08.836 <6>0[ 1213.971393] [   81]     0    81     1005       48   0     -16          -941 vold
05-06 14:45:08.836 <6>0[ 1213.971455] [   82]  1000    82       62        1   0     -16          -941 vcharged
05-06 14:45:08.836 <6>0[ 1213.971520] [   83]  1000    83      170       37   0     -16          -941 sprd_monitor
05-06 14:45:08.836 <6>0[ 1213.971587] [   86]     0    86    36180     4310   0       0             0 b2g
05-06 14:45:08.836 <6>0[ 1213.971649] [   87]  1001    87      205       45   0     -16          -941 rilproxy
05-06 14:45:08.836 <6>0[ 1213.971714] [   88]     0    88     1855       49   0     -16          -941 netd
05-06 14:45:08.836 <6>0[ 1213.971776] [   89]     0    89      173       37   0     -16          -941 debuggerd
05-06 14:45:08.836 <6>0[ 1213.971842] [   90]  1013    90     5371       84   0     -16          -941 mediaserver
05-06 14:45:08.836 <6>0[ 1213.971909] [   91]  1002    91      333       38   0     -16          -941 dbus-daemon
05-06 14:45:08.836 <6>0[ 1213.971975] [   92]  1017    92      433       37   0     -16          -941 keystore
05-06 14:45:08.836 <6>0[ 1213.972040] [   93]     0    93      319       45   0     -16          -941 nvm_daemon
05-06 14:45:08.838 94]  1000    94      461       44   0     -16          -941 modemd
05-06 14:45:08.838 <6>0[ 1213.972171] [   97]  1001    97     2419       49   0     -16          -941 rild_sp
05-06 14:45:08.838 <6>0[ 1213.972235] [  100]  1001   100      205       45   0     -16          -941 rilproxy
05-06 14:45:08.838 <6>0[ 1213.972300] [  103]     0   103      278       39   0     -16          -941 engmoded
05-06 14:45:08.838 <6>0[ 1213.972365] [  108]  1001   108     2419       47   0     -16          -941 rild_sp
05-06 14:45:08.838 <6>0[ 1213.972429] [  111]     0   111     1372       50   0     -16          -941 adbd
05-06 14:45:08.838 <6>0[ 1213.972492] [  154]  1000   154     7179       45   0     -16          -941 phoneserver_2si
05-06 14:45:08.838 <6>0[ 1213.972562] [  157]  1000   157     2103       47   0     -16          -941 engpcclient
05-06 14:45:08.838 <6>0[ 1213.972628] [  159]  1000   159      468       39   0     -16          -941 engmodemclient
05-06 14:45:08.838 <6>0[ 1213.972697] [  161]  1000   161      464       36   0     -16          -941 engservice
05-06 14:45:08.838 <6>0[ 1213.972763] [  317]     0   317     1320       77   0     -16          -941 slog
05-06 14:45:08.838 <6>0[ 1213.972825] [  343]     0   343    12644     1080   0       0             0 (Nuwa)
05-06 14:45:08.838 <6>0[ 1213.972890] [ 1793] 11793  1793    32579    10452   0       2           134 E-Mail
05-06 14:45:08.838 <6>0[ 1213.972955] [ 1823]  1010  1823      628       64   0     -16          -941 wpa_supplicant
05-06 14:45:08.838 <6>0[ 1213.973023] [ 1828]  1014  1828      229       51   0     -16          -941 dhcpcd
05-06 14:45:08.838 <6>0[ 1213.973088] [ 1879]     0  1879    14434     1340   0       1            67 (Preallocated a
05-06 14:45:08.838 <6>0[ 1213.973157] [ 2357]     0  2357      189       93   0       0             0 sh
05-06 14:45:08.838 <6>0[ 1213.973219] [ 2363]     0  2363     1116       21   0     -16          -941 adbd
05-06 14:45:08.838 <4>0[ 1213.973283] zram0 status unit(page):
05-06 14:45:08.838 <4>0[ 1213.973288]  	mem_used_total:  8108 
05-06 14:45:08.838 <4>0[ 1213.973294]  	compr_data_size: 7864 
05-06 14:45:08.838 <4>0[ 1213.973300]  	orig_data_size:  15624 
05-06 14:45:08.838 <4>0[ 1213.973306]  	num_reads:       72110 
05-06 14:45:08.838 <4>0[ 1213.973311]  	num_writes:      91982
triage: let's take this for 1.3T. 1.3T+ thanks
blocking-b2g: 1.3T? → 1.3T+
Summary: [Tarako][Email][LMK] Email gets killed when attach file to email → [Tarako][Email][LMK] Email attachment size limit logic bug results in too-large attachments being attached than detached rather than never detached. Results in email app getting killed.
:mcav, since you are currently overhauling the compose UI, you are best suited (like in Ghostbusters) to choose the form of the bit-rot trying to destroy you.
Attachment #8418377 - Flags: review?(m)
Comment on attachment 8418377 [details] [review]
instead of attaching the detaching, just do not attach in the first place

Looks good to me. My tarako is at the office, so I'm trusting that you've tested on-device. Sad about the historical l10n typo, but naming things is hard! :) Nice that this is a compact change.
Attachment #8418377 - Flags: review?(m) → review+
Tested on buri on master and tarako v1.3t.

== Landed on master
https://github.com/mozilla-b2g/gaia/pull/19000
https://github.com/mozilla-b2g/gaia/commit/564be21729cd2ad20c87f0d8ebf4671a7b9a22cc

== Landed on v1.3t (needed manual deconflicting and testing)
https://github.com/mozilla-b2g/gaia/pull/19013
https://github.com/mozilla-b2g/gaia/commit/e23c8dd8bddf4dbebcf01d283445cc94c05108c6


I think we probably want to uplift this to v1.4 so I'll request approval.
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
status-b2g-v1.3T: affected → fixed
status-b2g-v1.4: --- → affected
status-b2g-v2.0: --- → fixed
Resolution: --- → FIXED
Comment on attachment 8418377 [details] [review]
instead of attaching the detaching, just do not attach in the first place

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): bug 871897
[User impact] if declined: Files too large to send will be attached just to be detached again.  This is will produce a ton of I/O traffic and a good deal of memory usage.
[Testing completed]:  Tested on master and v1.3t builds.
[Risk to taking this patch] (and alternatives if risky): Risky to have the fix on trunk and v1.3t but not v1.4.
[String changes made]: None.  All strings already exist.
Attachment #8418377 - Flags: approval-gaia-v1.4?
Whiteboard: [sprd308061]
Attachment #8418377 - Flags: approval-gaia-v1.4? → approval-gaia-v1.4+
[Environment]
Gaia      fabc8154b31178515795b0a32068257fd5a16269
Gecko     https://hg.mozilla.org/releases/mozilla-b2g28_v1_3t/rev/a500e34d8819
BuildID   20140511164002
Version   28.1
ro.build.version.incremental=eng.cltbld.20140511.204401
ro.build.date=Sun May 11 20:44:08 EDT 2014


[Result]
PASS
Status: RESOLVED → VERIFIED
Hi Andrew, I tried on pvt build 20140513164002. Email still gets killed when add multiple attachment.

Here are the steps:
-------------------------------------------
1. Launch Email

2. Create a new mail

3. Add some attachments to the mail. 


Actual results:
-------------------------------------------
Sometimes, after I attached 3 pictures, email got killed.
Sometimes, after I attached 6 pictures, email got killed.
(Those pictures are taken with Camera, none of them is over 2MP.) 

Could you pls take a look?

p.s. I'd like to know that  there is a total size limit of the attachments.
Flags: needinfo?(bugmail)
(In reply to Bingqing Li from comment #11)
> Created attachment 8422232 [details]
> logcat-email-attach-pic
> 
> Hi Andrew, I tried on pvt build 20140513164002. Email still gets killed when
> add multiple attachment.
> 
Build Info
---------------------------------------------------------
Gaia      53e2fcfe410c50a4a6ee38d27e8a8590d8460e00
Gecko     https://hg.mozilla.org/releases/mozilla-b2g28_v1_3t/rev/61e4f8a91380
BuildID   20140513164002
Version   28.1
ro.build.version.incremental=eng.cltbld.20140513.203339
ro.build.date=Tue May 13 20:33:46 EDT 2014


It's more easier to reproduce this issue when attach pictures to the email.
(In reply to Bingqing Li from comment #11)
> 
> Hi Andrew, I tried on pvt build 20140513164002. Email still gets killed when
> add multiple attachment.
> 
> Here are the steps:
> -------------------------------------------
> 1. Launch Email
> 
> 2. Create a new mail
> 
> 3. Add some attachments to the mail. 
> 
> 
> Actual results:
> -------------------------------------------
> Sometimes, after I attached 3 pictures, email got killed.
> Sometimes, after I attached 6 pictures, email got killed.
> (Those pictures are taken with Camera, none of them is over 2MP.) 

I filed a new bug to track this issue: https://bugzilla.mozilla.org/show_bug.cgi?id=1010701
(clearing needinfo since this was addressed on the spun-off bug 1010701)
Flags: needinfo?(bugmail)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: