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

Out Of Memory Error in Import/Export Handler #3

Closed
GoogleCodeExporter opened this Issue Jun 26, 2015 · 3 comments

Comments

Projects
None yet
1 participant
@GoogleCodeExporter

GoogleCodeExporter commented Jun 26, 2015

Error report from the Android Market Developer Console:

Date reported: May 6, 2011 6:21:24 PM
Cryptnos version: 1.2.2

java.lang.OutOfMemoryError
at 
com.gpfcomics.android.cryptnos.ImportExportHandler$XMLFormat1Importer.run(Import
ExportHandler.java:1166)

User message: "Saud it was a false apo"

I have no idea what to make of the user message; sorry, but it's not 
descriptive and hard to understand. I'm not sure what "apo" is supposed to mean 
and the word "false" does not appear in any message text within the Cryptnos 
res/values/strings.xml resource, so this can't be quoting an error we displayed.

The line in question (1166 in ImportExportHandler.java) relates to the 
allocation of space for the decrypted XML after an export file has been 
imported.  Where "encryptedData" is the encrypted binary content of the file, 
"cipher" is the Bouncy Castle BufferedBlockCipher instance, and "plaintext" is 
a byte array to contain the decrypted XML:

byte[] plaintext = new byte[cipher.getOutputSize(encryptedData.length)];

It is certainly theoretically possible that there may not be enough RAM 
available to contain both the encrypted and decrypted data in memory at once. 
While there are checks to make sure that file to be imported does not exceed 
Integer.MAX_VALUE in size (i.e. it cannot be larger than 2GB), I doubt there 
are many (if any) Android devices that have 2GB of RAM available.

A quick and dirty fix would be to check and see if there's enough available RAM 
to hold both the encrypted and decrypted data. If there isn't, complain; if 
there is, do the decryption then free up the encrypted data so that RAM becomes 
available for the XML parser, which is the next step (and likely requires a 
good bit of memory as well).

Ideally, the decryption should be done in smaller chunks rather than on the 
entire file at once. I'll research this to see if there's a better way to 
handle it.

Quick notes:

To get available memory:
MemoryInfo mi = new MemoryInfo();
ActivityManager activityManager = 
(ActivityManager)getSystemService(ACTIVITY_SERVICE);
activityManager.getMemoryInfo(mi);

Original issue reported on code.google.com by jeff.darlington@gmail.com on 11 May 2011 at 4:51

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Jun 26, 2015

After a bit of research, I think I've got a handle on this. Going under the 
assumption that the above is true, that we're running out of space because 
we're trying to hold both the encrypted and decrypted data in RAM at the same 
time and the input data is just too large, I've made the following changes:

* I have added numerous statements during the import and export processes that 
null out objects such as ciphers, byte arrays, file streams, etc., immediately 
after they are no longer needed, rather than letting the garbage collector 
worry about cleaning them up. This will hopefully free up these allocations 
sooner.
* I have modified the import routines so that they no longer pull in the entire 
encrypted import file all at once. Rather, they allocate the entire plain text 
buffer as before (this is unavoidable), then read in a block of data at a time 
from the file and decrypt it. ("Block" in this instance refers to the block 
size of the cipher in question.) This should significantly reduce the amount of 
memory used during import. Unfortunately, this optimization cannot be done 
during export because the unencrypted data should never reside on persistent 
storage and must be contained in memory. If my suspicion about the original 
error is correct (see below), this hopefully won't be a problem.
* Before either operation, a check will be performed that will see if there is 
sufficient RAM available to hold the required data (both the entire encrypted 
and unencrypted buffers for export, and the small encrypted and larger 
unencrypted buffers during import). If there is insufficient RAM available, the 
user is warned via a Toast. They'll need to either free up RAM by killing 
processes, export a smaller number of sites, or try importing smaller files.

I'm still in the process of testing this bug so it's not ready to be released 
yet. Unfortunately, I'm not sure if I'll be able to reproduce it. None of the 
export files I've ever personally produced have been larger than a few 
kilobytes (and I have tons of sites saved), so I strongly suspect that the 
original reporter of the problem attempted to import a different type of file 
other than a valid Cryptnos export. Unfortunately, since the data is encrypted 
there is no way to tell whether or not the file is valid until the data has 
been decrypted, and if the user passed in a very large file, like an MP3 or 
video file, Cryptnos would try to read that in and decrypt it before it could 
discover that the file wasn't valid. Once I've verified that valid imports and 
exports play well with the new code, I'll try some invalid, very large files to 
see what I can come up with.

Original comment by jeff.darlington@gmail.com on 11 May 2011 at 8:27

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Jun 26, 2015

OK, testing is working out well. Using my original Motorola Droid (Android 
2.2.2), I was able to successfully export and import files with no problem. I 
then "accidentally" tried to import a 647MB MPEG-4 movie, well over the Droid's 
internal RAM. The new memory check worked as expected; it immediately aborted 
the import with an "insufficient memory" error.

I then attempted to import a 1.63MB ZIP file, also an invalid import. This was 
well with the RAM limit, so Cryptnos attempted to decrypt the file. It took 
quite a while to process, but eventually Cryptnos "decrypted" the file, found 
out that it was invalid, and produced an appropriate error message.

Since I broke out the import decryption process, I took the opportunity to add 
some progress messages to provide some additional feedback during that step of 
the process. Previously, this was just an opaque block; it would jump form 0% 
to 10% suddenly. Now it should count decryption as one third of the process and 
provide progress during this step. The primary motivation for this was the 
failed ZIP import described above; before, this long process seemed to lock up 
Cryptnos. (It wasn't really locked up, but it wouldn't respond to any form of 
cancellation.) With the new progress updates at least the user can tell the app 
is still working.

I'll run a few more tests, then I'll probably bump the version to 1.2.3 and 
send it to the Market.

Original comment by jeff.darlington@gmail.com on 12 May 2011 at 1:09

  • Changed state: Started
@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Jun 26, 2015

Found another bug, although I'm not sure if it was introduced by the fix here 
or if it existed prior to the change. The bug is minor and only affected 
imports of the old platform-dependent, version 1.0 export file format. All data 
was being imported correctly, but Cryptnos would report that the file was 
invalid. Since this may have been introduced during this bug fix, I'll list it 
here. Should be fixed now.

Original comment by jeff.darlington@gmail.com on 12 May 2011 at 2:05

  • Changed state: Fixed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment