Skip to content
This repository has been archived by the owner on Nov 1, 2022. It is now read-only.

Downloading 50mb+ files from thinkbroadband.com makes the browser to crash #6317

Closed
sv-sdeiac opened this issue Mar 19, 2020 · 2 comments
Closed
Assignees
Labels
🐞 bug Something isn't working 💥 crash <download> Component: feature-download
Milestone

Comments

@sv-sdeiac
Copy link

sv-sdeiac commented Mar 19, 2020

Steps to reproduce

  1. Go to thinkbroadband.com/download;
  2. Tap on the 50mb file;
  3. Tap on the "Download" button from the pop-up.

Expected behavior

The file starts to download without any problems.

Actual behavior

The app crash when downloading 50+mb files.

Device information

  • Android device: Google Pixel 3 (Android Q)
  • Reference Browser version: 1.0.2012(build #20761207)
  • Not reproducible using: Google Pixel 4 (Android Q), OnePlus 5T (Android 9) and LG g7 fit (Android 8.1).
    crash download.txt

┆Issue is synchronized with this Jira Task

@pocmo
Copy link
Contributor

pocmo commented Mar 19, 2020

2020-03-19 09:23:10.252 11107-13542/? E/SQLiteDatabase: Error inserting bucket_display_name=Download owner_package_name=org.mozilla.reference.browser parent=10 volume_name=external_primary _display_name=50MB.zip mime_type=application/zip _data=/storage/emulated/0/Download/50MB.zip _size=52428800 title=50MB group_id=1628528 is_download=true is_pending=1 date_added=1584602590 primary_directory=Download bucket_id=540528482 media_type=0 relative_path=Download/
    android.database.sqlite.SQLiteConstraintException: UNIQUE constraint failed: files._data (code 2067 SQLITE_CONSTRAINT_UNIQUE)
        at android.database.sqlite.SQLiteConnection.nativeExecuteForLastInsertedRowId(Native Method)
        at android.database.sqlite.SQLiteConnection.executeForLastInsertedRowId(SQLiteConnection.java:879)
        at android.database.sqlite.SQLiteSession.executeForLastInsertedRowId(SQLiteSession.java:790)
        at android.database.sqlite.SQLiteStatement.executeInsert(SQLiteStatement.java:88)
        at android.database.sqlite.SQLiteDatabase.insertWithOnConflict(SQLiteDatabase.java:1599)
        at android.database.sqlite.SQLiteDatabase.insert(SQLiteDatabase.java:1468)
        at com.android.providers.media.MediaProvider.insertFile(MediaProvider.java:2713)
        at com.android.providers.media.MediaProvider.insertInternal(MediaProvider.java:3254)
        at com.android.providers.media.MediaProvider.insert(MediaProvider.java:2903)
        at android.content.ContentProvider$Transport.insert(ContentProvider.java:309)
        at android.content.ContentProviderNative.onTransact(ContentProviderNative.java:154)
        at android.os.Binder.execTransactInternal(Binder.java:1021)
        at android.os.Binder.execTransact(Binder.java:994)
2020-03-19 09:23:10.279 15477-16911/? I/mozac/CrashReporter: Received crash: UncaughtExceptionCrash(throwable=kotlin.KotlinNullPointerException, breadcrumbs=[])
2020-03-19 09:23:10.279 15477-16911/? I/mozac/CrashReporter: Showing notification
    
    --------- beginning of crash
2020-03-19 09:23:10.292 15477-16911/? E/AndroidRuntime: FATAL EXCEPTION: DefaultDispatcher-worker-5
    Process: org.mozilla.reference.browser, PID: 15477
    kotlin.KotlinNullPointerException
        at mozilla.components.feature.downloads.AbstractFetchDownloadService.useFileStreamScopedStorage(AbstractFetchDownloadService.kt:446)
        at mozilla.components.feature.downloads.AbstractFetchDownloadService.useFileStream$feature_downloads_release(AbstractFetchDownloadService.kt:409)
        at mozilla.components.feature.downloads.AbstractFetchDownloadService$performDownload$1.invoke(AbstractFetchDownloadService.kt:342)
        at mozilla.components.feature.downloads.AbstractFetchDownloadService$performDownload$1.invoke(AbstractFetchDownloadService.kt:68)
        at mozilla.components.concept.fetch.Response$Body.useStream(Response.kt:79)
        at mozilla.components.feature.downloads.AbstractFetchDownloadService.performDownload$feature_downloads_release(AbstractFetchDownloadService.kt:338)
        at mozilla.components.feature.downloads.AbstractFetchDownloadService.startDownloadJob$feature_downloads_release(AbstractFetchDownloadService.kt:292)
        at mozilla.components.feature.downloads.AbstractFetchDownloadService$onStartCommand$1.invokeSuspend(AbstractFetchDownloadService.kt:205)
        at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33)
        at kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:56)
        at kotlinx.coroutines.scheduling.CoroutineScheduler.runSafely(CoroutineScheduler.kt:561)
        at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.executeTask(CoroutineScheduler.kt:727)
        at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.runWorker(CoroutineScheduler.kt:667)
        at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.run(CoroutineScheduler.kt:655)

@pocmo
Copy link
Contributor

pocmo commented Mar 19, 2020

I'll move this issue over to the AC repo since that should in theory affect every app using the feature-downloads component..

@pocmo pocmo transferred this issue from mozilla-mobile/reference-browser Mar 19, 2020
@pocmo pocmo added <download> Component: feature-download 🐞 bug Something isn't working 💥 crash labels Mar 19, 2020
@psymoon psymoon added this to ⏳ Sprint Backlog in A-C: Android Components Sprint Planning Mar 23, 2020
@csadilek csadilek moved this from ⏳ Sprint Backlog to 🏃‍♀️ In Progress in A-C: Android Components Sprint Planning Mar 24, 2020
csadilek added a commit to csadilek/android-components that referenced this issue Mar 26, 2020
csadilek added a commit to csadilek/android-components that referenced this issue Mar 26, 2020
@csadilek csadilek added this to the 38.0.0 🎪 milestone Mar 26, 2020
bors bot pushed a commit that referenced this issue Mar 26, 2020
6393: Closes #6317: Prevent inserting duplicate record into content resolver r=Amejia481 a=csadilek

OK, this hole was deep :). The cause of this crash is that we were unconditionally inserting into the content resolver which may already have a row / record of the download URI: #6317 (comment)

This can happen if a download fails or gets cancelled before we write the file. We will have a unique file name generated based on existing files, but also need to check if we have a record of the file in the resolver. If so, use it, otherwise create a new record.

I've tried for a few hours to write a meaningful test for this, but there are simply too many static methods involved here and the resulting refactoring was terrible: `ContentUris.withAppendedId`, `MediaStore.setIncludePending`, `MediaStore.Downloads.getContentUri` etc. That's properly the reason we don't have an existing test for this method :(

This also makes sure we now won't crash if for some reason there's another problem inserting into the content resolver, but fail the download instead.

Co-authored-by: Christian Sadilek <christian.sadilek@gmail.com>
@bors bors bot closed this as completed in 5597c3c Mar 26, 2020
A-C: Android Components Sprint Planning automation moved this from 🏃‍♀️ In Progress to 🏁 Done Mar 26, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
🐞 bug Something isn't working 💥 crash <download> Component: feature-download
Projects
No open projects
Development

No branches or pull requests

3 participants