Skip to content
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.

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

OOM error on some devices due to larger files is fixed. #4

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

abayomiAkanji
Copy link

The encryption and decryption method throw OOM error for larger files on certain devices (Example is Samsung Galaxy J7, Samsung S5...). The while loop and byte[] are being modified to fix this bug.

The while loop algorithm changed to fix OOM crash in larger files.
Byte[] size of the overloaded "Encryption" & "Decryption" methods are reduced to fixed the OOM error.
@codecov
Copy link

codecov bot commented Nov 10, 2017

Codecov Report

Merging #4 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master       #4   +/-   ##
=========================================
  Coverage     96.35%   96.35%           
  Complexity      108      108           
=========================================
  Files             3        3           
  Lines           357      357           
  Branches         47       47           
=========================================
  Hits            344      344           
  Misses            6        6           
  Partials          7        7
Impacted Files Coverage Δ Complexity Δ
alice/src/main/java/com/rockaport/alice/Alice.java 94.58% <100%> (ø) 87 <0> (ø) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 599fbcb...c650dc5. Read the comment docs.

@rockaport
Copy link
Owner

It's odd that this change fixes an OOM error. Apps generally have +64MB of heap/ram/memory allotted and these file IO buffers are only 4kB. I could see how if you ran this in parallel on multiple files you might run into a problem, but the library isn't threadsafe anyway so you shouldn't do that.

The only way I can see this throwing an OOM error is if you tried to load a large file into memory and use the byte array encrypt/decrypt routines. Changing the buffer sizes wouldn't help you there and instead give me the feeling that there's a different problem or memory leak happening.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants