-
Notifications
You must be signed in to change notification settings - Fork 182
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
Comments
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. |
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 () { }; |
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 |
Be that as it may - I've observed something very strange in your reproducer. Why is |
…gOnInit()` simultaneously; #2077 reduce the required version of RxJS to 6.0
…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)
…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)
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. |
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 |
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:
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.
Link to reproduce project https://drive.google.com/file/d/16M1YVELtXcG-jhnpxf-4xNXUtIP-3foG/view?usp=sharing
The text was updated successfully, but these errors were encountered: