Created attachment 597452 [details] testcase Steps to reproduce: - Visit testcase, tap on the button (which makes a really large url after the '#' part). Result: crash I noticed on various phones that I tested that not only Fennec crashes, but it seems to crash after every restart, unless I clear all data. Tested on the HTC Desire HD, LG Optimus Black an Samsung Galaxy Nexus (ICS Fennec doesn't give a crash reporter?) https://crash-stats.mozilla.com/report/index/bp-cf30c609-f296-4751-8aa4-ab8872120215 0 libmozglue.so malloc_print_stats memory/jemalloc/jemalloc.c:2164 1 @0x0 2 libmozglue.so getanswer other-licenses/android/getaddrinfo.c:1419 3 libxul.so nsRegion::SetToElements gfx/src/nsRegion.cpp:330 4 libxul.so nsRegion::SetEmpty gfx/src/nsRegion.h:173 5 libxul.so nsRegion::SubRegion gfx/src/nsRegion.cpp:1070 6 @0x45abbf86 7 libxul.so nsDisplayListBuilder::SubtractFromVisibleRegion nsRegion.h:107 8 libxul.so nsDisplayList::ComputeVisibilityForSublist nsTArray.h:480
This was on current mozilla-central nightly.
To be clear, the "really large url" in this test case is over 2 million characters in length. There are no problems if the length is reduced to, for example, 4000 characters. The test case consistently crashes for me, but I have not seen this particular stack trace yet in my testing. Instead, I see crashes on database operations in BrowserProvider.updateHistory(). I don't fully understand what fails in updateHistory, but if I skip that code, Fennec still crashes with an OOM exception when the URI is copied or manipulated. An obvious solution is to restrict the size of the URL by truncating input -- is that a reasonable approach? There is some info on max url lengths for browsers here: http://www.boutell.com/newfaq/misc/urllength.html
limiting the URL to something less than 2 million chars sounds fine to me. let's just be sure that using a shorter URL doesn't just take longer to crash, after repeatably loading the URL. Can you test 64k, 128k, 256k limits and see how those work?
Created attachment 606807 [details] [diff] [review] patch to truncate uri in json - for discussion only This patch truncates the uri and documentURI fields of JSON messages and avoids the crashes we were seeing with this test case. 2 problems remain: 1. Clicking the button additional times still results in OOM. I think this is because of Gecko allocations (the script keeps doubling its string regardless of this truncation, so the 2nd click generates a 4MB string, the 3rd an 8MB string, etc). 2. Even on the first click, I see evidence of database operations on the 2MB data; I don't know where this is coming from: E/CursorWindow(32279): need to grow: mSize = 2097152, size = 2097179, freeSpace( ) = 2092635, numRows = 4 E/CursorWindow(32279): not growing since there are already 4 row(s), max size 20 97152 E/Cursor (32279): Failed allocating 2097179 bytes for text/blob at 3,1 D/Cursor (32279): finish_program_and_get_row_count row 0 Looking for suggestions for a better (earlier) place to detect and truncate very large input...
Created attachment 609794 [details] [diff] [review] truncate uri in browser.js The approach used here is straight-forward: truncate uri fields before sending to Java. With this patch, the test case passes, and the button can be clicked about 3 times (on my Galaxy S) before the OS kills Fennec for excessive memory use. (Fennec does not report an exception...it is killed cleanly.) Note that the test is additive: first click creates a 2MB string, second click a 4MB string, 3rd click a 8MB string...so this result seems reasonable. I chose a limit of 2048 characters because that limit is already used in, for example, http://hg.mozilla.org/mozilla-central/file/0ff816e5e992/dom/src/offline/nsDOMOfflineResourceList.cpp#l84 ... but much larger values (eg. 256K) are equally effective in addressing this bug. I am concerned that this solution seems "fragile" - truncation code is duplicated and needs to be carried over to any new messages containing uri fields. Suggestions for a different approach welcome!
Since the patch to limit the URL still allows a crash - an expected crash given the huge string - I think we should WONTFIX this. Thoughts?
I'm going to WONTFIX after a quick discussion on IRC and looking at the URL limit in Firefox and other browsers: http://www.boutell.com/newfaq/misc/urllength.html Getting OOM killed for trying to allocate more than the devices memory will happen.