Infinite loop hanging megathread (minPresenceAhead, margin, break)

[tom2strobl]

Description

1. There has to be a general flaw in the layout logic resulting in infinite loops that crash browser engines (and whatever the issue might be it is evident that crashes are the worst expression of failure handling)
2. (maybe related or not) minPresenceAhead works on the simplest examples but fails to work for me in any real-life scenarios where nested Views (eg inevitable for rendering bullet lists) and page padding (necessary for my case) are used

Note that I use 3.3.8 which afaik should already include #2400

Preliminary work

Regarding 1)

• SVG that overflows the page causes a hang #1384 is related, simply nothing really happened since 2021
• Too big margin makes hang-up CPU. #2083 was closed (because OP gone rogue) but seems to be related
• minPresenceAhead crashed application #2303 was closed but seems to be related
• Hangs when Image goes off page #2242 might be related
• Hangs when wrap={false} too big for one page #874 might have been related

Regarding 2)

• minPresenceAhead with Row and Column Layout - Children break #2595 absolutely is related and showcases it pretty well
• minPresenceAhead - calculating the meaning of 'presence' on next sibling element #955 seems it doesn't work for a lot of folks for a very long time

Reproducable cases

(note I'm using @jeetiss REPL for hanging issues since you obviously can't share an url of the official REPL if the process hangs)

1A) Simplest reproducible case REPL: (yes, this is still a thing, nobody confirmed it's fix really)

ReactPDF.render(<Document><Page><Text style={{ paddingBottom: 1000 }}>Text</Text></Page></Document>);

1B) Reducing available space to make it not fit REPL

1C) It's not padding-bound either, as decreasing padding, but increasing font-size has the same effect REPL

1D) In my production app I also have a case (it's a larger document and more complex case that I can't 1:1 easily copy over to a REPL) where marking one of the <View>s with break={true} also suddenly introduces the infinite loop hanging. Changing any wrap or minPresenceAhead props don't affect the outcome, yet changing some margins does (although I was not able to find a real pattern yet unfortunately). So I think it's not the implementation of wrap/minPresenceAhead per se but likely a more general flaw in the layout (splitting?) logic. I'll try to spend some more time somehow getting this into a REPL. My gut says that some certain composition of Views with certain paddings/margins (and/or page padding) is getting too large for the Page and then in the splitting logic it somehow falls into the infinite loop hole.

2A) A more contrived Real-life scenario REPL with the minPresenceAhead issue
I removed any fancy logic, but kept a lot of styling on purpose so there's something more elaborate to test again when we're confirming a fix.

Thoughts

Interestingly in my hundreds (?) of manual tests I never saw this:

react-pdf/packages/layout/src/steps/resolvePagination.js

Line 44 in fab09cc

`Node of type ${node.type} can't wrap between pages and it's bigger than available page height`,

@carlobeltrame Does

react-pdf/packages/layout/src/node/shouldBreak.js

Line 29 in fab09cc

const shouldBreak = (child, futureElements, height) => {

work recursively in a sense that the size and margin/paddings of child Texts of Views of Views are taken into accout? Also – is page size and padding taken into account for the bounding box of the flow? I am asking because of this comment:

react-pdf/packages/layout/src/node/shouldBreak.js

Line 38 in fab09cc

// (as long as react-pdf does not support breaking into differently sized containers)

What I mean is, if the page has paddingBottom (green) and minPresenceAhead is within the canvas but inside the padding then maybe the following siblings are not taken into account correctly(?):

I understand that this GH issue might actually be multiple issues at once, but I have the feeling that there must be some general connection.

Desktop (please complete the following information):

• OS: MacOS
• Browser: nodejs (electron main thread, but happens in chrome REPL as well)
• React-pdf version 3.3.8

[tom2strobl]

So regarding my main issue 1D) I've dug around for 6 hours without a definite conclusion.

Total weirdness ensures:

• I've patched asyncCompose to log the name of the async fn that will be executed to find out where it's stuck looping and it consistently prints resolveAssets as the last line, from which one would like to believe it's stuck there and never reaches resolveSvg(the next step in the chain)
• here's where it gets spicy: commenting out the fontFamily declaration on the (very often used) paragraph <View> makes the document render, even though it loads

  react-pdf/packages/layout/src/steps/resolveAssets.js

  Line 28 in fab09cc

  promises.push(fontStore.load(n.style));

  the same amount font files (2 font files) and without issues (the fontfiles are fine, the fontkit font loading is fine, the buffers are complete and everything, at the end of the day this works for all other 99% of documents so why would it suddenly not work)
• now if you thought I was onto something – hell no: keeping the fontFamily but instead commenting out a marginBottom (yes, a margin) of the bullet list wrapper <View> (oddly specific) makes the document render
• same goes for setting all break properties to false, renders flawlessly

It really looks like a certain vertical flow position of a certain View or certain Text results in the infinite loop and the margin and different font size or breaking just contribute to that awkward spot.

I'm inclined my logging strategy doesn't work as a marginBottom can't have an effect on resolveAssets and instead it breaks somewhere else but the logs don't reach stdout due to infinite loops preventing the next tick that would print the log. If anyone has an idea how I could go about finding where the infinite loop occurs I'd be grateful.

[jakeprem]

I'm seeing roughly the same issue as well. We're rendering a list of entry components (elements in a medical record). Certain combinations will cause ReactPDF to go into an infinite loop in the resolvePagination step. In a flame graph, relayout ends up being the function called over and over.

For one of our problem documents:

• Shuffling the the entries fixes the infinite loop
• Removing all bottom margins fixes it
• Removing the if (shouldBreak) {} block from splitNodes fixes it.
• When I log each of the if blocks in splitNodes, I see that the loop initially goes into the shouldSplit and isOutside blocks, but eventually gets caught in the shouldBreak block and then always goes back into that one.

Like @tom2strobl said above, it seems like the margins might be a red herring and the bug is actually caused by a node with specific attributes being split.

I'm continuing to investigate the page splitting logic to see where the loop might be happening. Apologies for the relative lack of details and clarity in my comment 😁

---

• We're running in Node.js; I'm testing against a local Express server
• I've linked my test setup to a local copy of the react-pdf library so testing against the latest commit in master

[jakeprem]

Did a bunch of digging and debugging into this.

I believe the root cause is shouldBreak.js is causing nodes to be broken that would fit in the page on their own, but not with their bottom margin included. When there are no sibling nodes present, then the lower value (sometimes -Infinity) will be used, preventing this node from breaking. Otherwise the result of getEndOfMinPresenceAhead is used which is child.box.top + child.box.height + child.box.marginBottom + getMinPresenceAhead(child) (minPresenceAhead = 0 in my case). So if the sibling nodes are smaller than the current node, the bug is prevented, if they are larger then the bug is possible.

We have paddingTop: 35 set on our pages which causes the first node after a relayout to have child.box.top == 35 (represented as 99 in the example below)

Example (in the form of params to shouldBreak):

• Height: 1000
• child.box.top: 99
• child.box.height: 900
• child.box.marginTop: 0
• child.box.marginBottom: 10
• futureElements: Anything with at least one child where box.top + box.height > 1009

1. shouldSplit == false -> height < child.box.top + child.box.height -> 1000 < 99 + 900
2. endOfPresence == 1009 -> code here
3. breakingImprovesPresence == true -> child.box.top > child.box.marginTop -> 99 > 0

• !shouldSplit && endOfPresence > height && breakingImprovesPresence -> true && 1009 > 1000 && true

These conditions cause shouldBreak to return true. But when this child breaks, resolvePagination.js will push it and all following nodes to the next page (in the splitNodes function), while resetting its top. On the next loop, the first node will have its top reset. In this iteration, nothing on this node has changed to it will again trigger shouldBreak == true, and again gets pushed to the next page. This causes there to always be a nextPage in the paginate function, so while nextPage !== null will keep looping infinitely, as it will never split the problem node, and thus progress through the document.

Workaround

Using paddingBottom instead of marginBottom seems to work ok, but has some tradeoffs, especially if you're using background colors.

Suggested Fix

• Does getEndOfMinPresenceAhead in shouldBreak.js really need to include the bottom margin of the node if we're already at the end of the page anyway?
• Should breakingImprovesPresence have similar logic for the bottom margin as it does for the top margin, only returning true if the extra bottom space is truly needed?

[kasper573]

I'm also running into an infinite loop hanging and ultimately crashing the browser. I don't have much to add except that I tried removing any and all bottom margin in my entire document, and I still got the infinite loop hanging. Was also using React-pdf version 3.3.8 on chrome, and I was not using any minPresenceAhead.

Edit: I solved my problem. It seems that my issue was that I had a Page element whose first child was a View that had break enabled. I was unable to reproduce the issue in REPL though, so it's not as simple as adding a break like that. It's likely related to some of the other stuff I have in my document, but removing the break did fix it for me. Figured I'd share this info.