PDFium for Web Assembly is having major memory leak issues

[Cetin Sert]

(PDFium has just recently been brought to web via WASM thanks to the people involved in this thread: https://groups.google.com/g/pdfium/c/xqMSoBa6ZVU/m/2NYU5fJABAAJ.)

There are still some major memory leak issues going through mmap and munmap.

Can some developers please review these

Issue with FPDFBitmap_Destroy(FPDFBitmap_Create*(496, 496, 4))

https://github.com/emscripten-core/emscripten/issues/14459#issuecomment-864373169 (perhaps from this point downward)

https://github.com/emscripten-core/emscripten/issues/14459

Issue with FPDF_RenderPageBitmap()

https://user-images.githubusercontent.com/27027/122643367-6bb8f280-d14a-11eb-9acb-40d289dcfc0e.png (screenshot)

https://github.com/paulo-coutinho/pdfium-lib/issues/33#issuecomment-863707842 (post)

and give us an idea on how to move forward?

[Lei Zhang]

On the first issue, you wrote "munmap sometimes (...) fails to call
free". I'm not sure how PDFium developers can help here. If PDFium
calls munmap(), and the munmap() implementation does not fulfill its
end of the contract, that's their problem to fix, right?

> --
> You received this message because you are subscribed to the Google Groups "pdfium" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to pdfium+un...@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/pdfium/a27004c8-d942-4389-909d-76a2f1f3122bn%40googlegroups.com.

[Cetin Sert]

You are right but one question!

Does PDFium ever do partial munmap() calls, as was noticed with the port here:
https://github.com/emscripten-core/emscripten/issues/14459#issuecomment-864400904

or is that a bug introduced by our use of emscripten/em++?

Thanks

[Lei Zhang]

I think this is the only code in PDFium that calls munmap() in that
way: https://pdfium.googlesource.com/pdfium.git/+/cb20bfc8/third_party/base/allocator/partition_allocator/page_allocator_internals_posix.h#129

On Mon, Jun 21, 2021 at 11:21 PM Cetin Sert <cetin...@gmail.com> wrote:
>
> You are right but one question!
>
> Does PDFium ever do partial munmap() calls, as was noticed with the port here:
> https://github.com/emscripten-core/emscripten/issues/14459#issuecomment-864400904
>

> To view this discussion on the web visit https://groups.google.com/d/msgid/pdfium/0702ba76-8161-4d31-97e7-3134ca95eec2n%40googlegroups.com.

[Cetin Sert]

Thank you for the prompt response!

Is my interpretation correct that the corresponding Windows file

https://pdfium.googlesource.com/pdfium.git/+/cb20bfc8/third_party/base/allocator/partition_allocator/page_allocator_internals_win.h#52

suggests a potential patch for WASM builds as Windows (like emscripten / em++) also cannot resize the allocation run (=? perform partial munmap) and simply frees whole pages first before re-allocating instead?

[Lei Zhang]

Certainly worth a try.

> To view this discussion on the web visit https://groups.google.com/d/msgid/pdfium/7998fa7b-5777-4475-b2ab-ef419a117969n%40googlegroups.com.

[Cetin Sert]

I have tried this as described here: https://github.com/emscripten-core/emscripten/issues/14459#issuecomment-866066268 (only replace the the body of TrimMappingInternal from _posix.h with that of _win.h). It seems to help in the most basic cases but it does hang when rendering pages. Is there any place else I should look at / replace?

[Cetin Sert]

Emscripten developers believe we need a different approach (one of their message: https://github.com/emscripten-core/emscripten/issues/14459#issuecomment-866250673)

> I don't think that will work. The main reason is that the hint argument which is the first argument to SystemAllocPages is ignored under emscripten. As I said, I think you are going to want a more specific, higher level, solution that avoids SystemAllocPages completely and uses something like posix_memalign instead. Hopefully this can be done upstream: https://groups.google.com/g/pdfium/c/cDkppTy9mEc.

[Cetin Sert]

Also of note is this message which details the incompatibilities in current _posix.h with emscripten:
https://github.com/emscripten-core/emscripten/issues/14459#issuecomment-866053919 

[Lei Zhang]

If you can work out a reasonable approach with the Emscripten
developers, PDFium can accept a patch to do memory management
differently with WASM.

> To view this discussion on the web visit https://groups.google.com/d/msgid/pdfium/819abcd2-3453-4229-b33f-b44a1a72842fn%40googlegroups.com.