Skip to content

Fix OutOfMemoryError during media type sniffing #44

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 2 commits into from
Nov 8, 2021
Merged

Fix OutOfMemoryError during media type sniffing #44

merged 2 commits into from
Nov 8, 2021

Conversation

@mickael-menu
Copy link
Member

@mickael-menu mickael-menu commented Nov 5, 2021

We got some reports of OutOfMemoryError crashes while sniffing a file as a string:

Screen Shot 2021-11-05 at 12 07 31

@mickael-menu mickael-menu requested a review from qnga November 5, 2021 11:08
qnga
qnga reviewed Nov 8, 2021
View reviewed changes
Copy link
Member

@qnga qnga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Is

kotlin-toolkit/readium/shared/src/main/java/org/readium/r2/shared/util/mediatype/SnifferContent.kt

Line 38 in 966d1d0

// We only read files smaller than 5MB to avoid an [OutOfMemoryError].
still useful though?

@mickael-menu
Copy link
Member Author

mickael-menu commented Nov 8, 2021

@qnga Good question, I think it can't hurt to prevent going into a OutOfMemoryError in the first place since when we use this, it's always to read a text file. It should improve performance too.

@mickael-menu mickael-menu merged commit b5b2b73 into develop Nov 8, 2021
@mickael-menu mickael-menu deleted the fix/oom-sniffer branch November 8, 2021 11:20
mickael-menu added a commit to demarque/readium-kotlin-toolkit that referenced this pull request Nov 8, 2021
@mickael-menu
Squashed commit of the following:
dfc82b4 
commit b6b41ee
Author: Mickaël Menu <mickael.menu@gmail.com>
Date:   Mon Nov 8 18:37:21 2021 +0100

    Add `EdgeTapNavigation` to trigger page turns on edge taps

commit b5b2b73
Author: Mickaël Menu <mickael.menu@gmail.com>
Date:   Mon Nov 8 12:20:25 2021 +0100

    Fix OutOfMemoryError during media type sniffing (readium#44)

commit 7b64b93
Author: Mickaël Menu <mickael.menu@gmail.com>
Date:   Mon Nov 8 11:06:52 2021 +0100

    Add a fallback when Chrome Custom Tabs are not available (readium#43)

commit 01a88a8
Author: qnga <32197639+qnga@users.noreply.github.com>
Date:   Thu Nov 4 14:37:22 2021 +0100

    Enable source and doc publishing (readium#42)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@qnga qnga qnga approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@mickael-menu @qnga