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

Too big margin makes hang-up CPU. #2083

Closed
fmy opened this issue Nov 15, 2022 · 10 comments
Closed

Too big margin makes hang-up CPU. #2083

fmy opened this issue Nov 15, 2022 · 10 comments

Comments

@fmy
Copy link

fmy commented Nov 15, 2022

Describe the bug
Too big margin styles like 1000 make rendering freeze.

To Reproduce
Steps to reproduce the behavior including code snippet (if applies):

  1. Go to this repl
  2. Change marginTop to 1000 from 100.
  3. Browser hang-up

Expected behavior
At least it should crash or timeout.

Screenshots
スクリーンショット 2022-11-15 18 22 38

Desktop (please complete the following information):

  • OS: [e.g. MacOS, Windows] both
  • Browser [e.g. chrome, safari] any browser and node environment
  • React-pdf version [e.g. v1.1.0] @react-pdf/renderer@3.0.1 & @react-pdf/renderer@2.1.1
@oviniciuslara
Copy link

I have reproduced this as well

@jeetiss
Copy link
Collaborator

jeetiss commented Jan 2, 2023

the same with fontSize

@oviniciuslara
Copy link

@jeetiss this seems to be a generic problem with everything that goes over the page space, there is a lot of similar issues.

The root cause of this should be tracked down to fix this permanently. Do you have any idea where the problem lay down?

@oviniciuslara
Copy link

I saw two different problems with this package:

  1. Crashes if some element is positioned out of the page or if it overflows the page.
  2. Crashes if the actual page has a lot of content, thus using a lot of memory.

For static pages I think just adding a checker to verify if the element is inside the page is enough to avoid this situation, the problem is with dynamic content that could lead to the same situation. At least the package should throw an intuitive log about it, not crashing the entire browser.

About the memory limit we should also throw an intuitive log and ignore further elements before reached a memory limit.

Also, this is just a workaround based on simple analysis, accordingly to the root cause of this CPU crasher the solution may be different.

@oviniciuslara
Copy link

@diegomura do you have any knowledge about this that could help?

I consider this issue a VERY HIGH since it affects rendering, the core of this package.

@jeetiss
Copy link
Collaborator

jeetiss commented Jan 9, 2023

Yet another example of rendering freeze

@kdubb1337
Copy link

Related issue for me. I have a very long View that should be page wrapped, which works fine. But then when I add any padding to the containing Page it hangs.

@bezchristo
Copy link

Getting the same issue when the image doesn't fit on one page.

@carlobeltrame
Copy link
Contributor

It's possible that the long element freeze could be fixed in #2400. I found some problems (including some potential infinite loops) in the page breaking algorithm there and was able to fix them. If anyone has the means to test that PR, would be highly appreciated.

@diegomura
Copy link
Owner

Thanks @carlobeltrame . Since your PR change this logic considerably and we havent' heard again about this I'll close it. It can be opened again if someone says this still happens

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

No branches or pull requests

7 participants