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

Memory is not released after parent component of the pdf viewer is destroyed #2329

Closed
solita-PerttuLe opened this issue May 8, 2024 · 7 comments
Assignees
Labels
21 bug Something isn't working partially solved The issue is solved, or has at least improved, but it's still not perfect.

Comments

@solita-PerttuLe
Copy link

solita-PerttuLe commented May 8, 2024

After navigating away from a component containing pdf viewer carbage collector does not release memory allocated to viewer. All instances are still alive. This makes also parent component to stay in memory. I don't have any lingering subscriptions or other memory leaks. There is possibility that the issue ties with the base component Mozilla pdf viewer but I don't know how to find it out.

I provided a sample project. There is no pdf included but the error occurs still.

Steps to reproduce:

  1. Open project
  2. Run npm install
  3. Run npm start
  4. Open localhost:4200/viewer
  5. Navigate couple of times between 2 views by pressing navigate buttons
  6. Take heap snapshot with browsers memory profiler tool

Below is screenshot after navigating 3 times between views.
Also viewer component instance occurs three times but another component only once as there is no viewer embedded.
Removing viewer from DOM programmatically before destroying parent component does not help. If this is related to Mozilla base component is there any hint what could cause this.

memory_heap

Link to reproduce project https://drive.google.com/file/d/16M1YVELtXcG-jhnpxf-4xNXUtIP-3foG/view?usp=sharing

@stephanrauh stephanrauh self-assigned this May 8, 2024
@stephanrauh stephanrauh added bug Something isn't working Stephan wants to look at it! labels May 8, 2024
@stephanrauh
Copy link
Owner

That sounds disturbing. Thanks a lot for providing a detailed error description and a reproducer project! I'm afraid you have to wait at least two weeks because I'm busy at the moment, but I'll put the ticket onto the top of the pile.

BTW, I've downloaded the file, you can remove it if you need to free space.

@solita-PerttuLe solita-PerttuLe changed the title Memory is not released after parent component of the pdf viewer is destoryed Memory is not released after parent component of the pdf viewer is destroyed May 14, 2024
stephanrauh added a commit that referenced this issue May 25, 2024
@stephanrauh
Copy link
Owner

I've started to get rid of the memory leaks, but it seems to be difficult. For example, Chrome claims this line holds a pointer to "this" - but I don't get it, it's just an empty function without any reference to the surrounding context:

 PDFViewerApplication.pdfLinkService.setHash = function () { };

@stephanrauh
Copy link
Owner

I've achieve some progress with version 20.5.0-alpha.0 - but I'm afraid the memory leak is still there. Only the "distance" has increased, which means it's more difficult to find the rest of the problem:
image

stephanrauh added a commit that referenced this issue May 30, 2024
… unlock Angular 18; #2329 embrace signals to reduce the memory leak (work in progress - the leak is still there)
stephanrauh added a commit to stephanrauh/pdf.js that referenced this issue May 30, 2024
@stephanrauh
Copy link
Owner

I suspect solving this memory leak is going to to take a long time. I thought I was pretty proficient at solving memory leaks, but at the moment, I have a hard time understanding the heap snapshots. It's obvious I'm getting closer, but I'm afraid I didn't manage to release many bytes yet. In other words, my progress doesn't show in your application.

Also, I'm not sure what to to about the base library. It's meant to be the PDF viewer of PDF, so embedding in an application isn't the use-case of the library. As a consequence, they probably don't care much about memory leaks (or at least, not about this kind of memory leaks). It may be difficult to remove objects like PDFViewer, PDFDocumentProxy, and so on from memory.

@stephanrauh
Copy link
Owner

Be that as it may - I've observed something very strange in your reproducer. Why is Promise.withResolvers undefined? This forces the viewer to load the slow ES5 files - but that shouldn't be necessary in a modern Chrome browser. What's going on here?

stephanrauh added a commit that referenced this issue Jun 19, 2024
stephanrauh added a commit that referenced this issue Jun 19, 2024
…gOnInit()` simultaneously; #2077 reduce the required version of RxJS to 6.0
stephanrauh added a commit that referenced this issue Jun 24, 2024
… process of loading and initializing so that switching between the demos also works on Safari (plus reduce the memory leak reported in #2329): #2387 stop the animation loop when destroying the `PageFlip` object

stephanrauh/ngx-extended-pdf-viewe
stephanrauh added a commit to stephanrauh/pdf.js that referenced this issue Jun 24, 2024
…the global namespace and modify the process of loading and initializing so that switching between the demos also works on Safari (plus reduce the memory leak reported in stephanrauh/ngx-extended-pdf-viewer#2329)
stephanrauh added a commit to stephanrauh/pdf.js that referenced this issue Jun 24, 2024
…the global namespace and modify the process of loading and initializing so that switching between the demos also works on Safari (plus reduce the memory leak reported in stephanrauh/ngx-extended-pdf-viewer#2329)
@stephanrauh stephanrauh added the 21 label Jun 27, 2024
@stephanrauh
Copy link
Owner

I suspect the memory leak has become better. Of course, it's far from being solved, but updating to version 20.5.2 might reduce the pain.

@stephanrauh stephanrauh added partially solved The issue is solved, or has at least improved, but it's still not perfect. and removed Stephan wants to look at it! labels Jul 28, 2024
@stephanrauh
Copy link
Owner

Version 21.0.0-beta.1 ships with more improvements. Let's close the ticket for now. I don't think the memory leak is gone, but I believe it's improved to a degree that many developers (and users) can live with it. I'll keep the issue in my mind, and I hope I'll manage to find the root cause soon.

Note that now the JavaScript files are loaded in a service. So now you have to destroy both the component and the service to find out whether there's a memory leak. The advantage of using a service is that the huge JavaScript files are only loaded once. So there's no danger that countless instances of PDFViewerApplication pile up and block all available memory.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
21 bug Something isn't working partially solved The issue is solved, or has at least improved, but it's still not perfect.
Projects
None yet
Development

No branches or pull requests

2 participants