Revert "Fix memory leak by freeing owned image bytes after reading barcode"

[phlip9]

Hey, just noticed the recent commit. Sadly it appears to introduce a double-free and/or allocator mismatch, so I've added this commit to revert. I know touching this FFI code is pretty nasty -- I wish there was an easy way to test the code with sanitizers or valgrind or something, but I think that would require recompiling the whole dart VM toolchain w/ sanitizers enabled... Until then, feel free to cc me on any PRs that touch native code. I'm happy to review : D

Commit

This reverts commit e187556f9a355056b8a30733fbf67bc4f64f8e9b.

1. The DecodeBarcodeParams struct already has a destructor that frees the .bytes field.
2. Memory needs to be free'd with dart_free and not delete[], as libstdc++ may use a different allocator.
3. The params arg here is passed as a borrow (const DecodeBarcodeParams&) which should signal that the function is not meant to manage any of the resources in params.

[khoren93]

Hey @phlip9,

Thank you for pointing out the issues in the recent commit. After the changes from this Pull Request, I have noticed a memory leak that wasn't present before. This leak occurs specifically on iOS, while there is no leak on Android. I've attached screenshots from Xcode showing how memory consumption increases over time on iOS devices after these changes.

Here are the screenshots for reference: Before

After

If you have the capability, could you please test this on an iOS device?

Thanks again for your assistance!

[phlip9]

Huh, yeah that definitely looks like a memory leak. Good catch! I wonder if the Xcode toolchain is reading native_zxing.h as a C header and not including the destructor? Super weird... I'll take a look in an hour or so when I'm at my work laptop :+1:

[phlip9]

Ok looks like I forgot to call the destructor in dart_deleter... Thought unique_ptr would handle that for me, but apparently not!

Anyway, can confirm that the leak disappears with the latest commit:

Before	After