Skip to content

Add guard against NPE#37

Merged
AndroidDeveloperLB merged 1 commit intoAndroidDeveloperLB:masterfrom
cassgenerator:master
May 15, 2018
Merged

Add guard against NPE#37
AndroidDeveloperLB merged 1 commit intoAndroidDeveloperLB:masterfrom
cassgenerator:master

Conversation

@cassgenerator
Copy link
Copy Markdown
Contributor

It is possible that GetDirectBufferAddress returns null in some cases. Given this it would be best to guard against attempting to access the _storedBitmapPixels variable on a null pointer.


I could only trigger this issue in a kinda race condition in my application, when I would switch to a new view while an Async task was operating on JniBitmaps. I think this caused the underlying memory to be cleaned up by the system and it would then crash when trying to get the bitmap next.

As far as I can tell this guards against that issue, it no longer crashes for me. However it has been a while since I've done any C++. So let me know if anything needs changing.

It is possible that `GetDirectBufferAddress` returns null in some cases. Given this it would be best to guard against attempting to access the `_storedBitmapPixels` variable on a null pointer.
@AndroidDeveloperLB
Copy link
Copy Markdown
Owner

I think you are correct:
https://docs.oracle.com/javase/7/docs/technotes/guides/jni/spec/functions.html#GetDirectBufferAddress

Returns the starting address of the memory region referenced by the buffer. Returns NULL if the memory region is undefined, if the given object is not a direct java.nio.Buffer, or if JNI access to direct buffers is not supported by this virtual machine.

@AndroidDeveloperLB AndroidDeveloperLB merged commit 97a16c2 into AndroidDeveloperLB:master May 15, 2018
@AndroidDeveloperLB
Copy link
Copy Markdown
Owner

Please also check if there are similar cases that I didn't protect against.

@cassgenerator
Copy link
Copy Markdown
Contributor Author

I'm chasing another crash, perhaps it is related. Hopefully I track it down.

@AndroidDeveloperLB
Copy link
Copy Markdown
Owner

Thank you for contributing.

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