mdn / yari

The platform code behind MDN Web Docs
Mozilla Public License 2.0
1.19k stars 502 forks source link

Memory OOM on /en-US/docs/Web/API/MediaSession/setActionHandler #1270

Closed peterbe closed 4 years ago

peterbe commented 4 years ago

The file that triggers the OOM crash is: https://github.com/mdn/content/blob/e7c50f5d2383924101236cbcedaf218648f311b9/files/en-us/web/api/mediasession/setactionhandler/index.html

To reproduce, simply open http://localhost:3000/en-US/docs/Web/API/MediaSession/setActionHandler in yarn dev After a long time, you'll get something like this:

FATAL ERROR: Ineffective mark-compacts near heap limit Allocation failed - JavaScript heap out of memory
 1: 0x100bfea43 node::Abort() (.cold.1) [/usr/local/Cellar/node/14.5.0/bin/node]
 2: 0x10008609d node::FatalError(char const*, char const*) [/usr/local/Cellar/node/14.5.0/bin/node]
 3: 0x100086206 node::OnFatalError(char const*, char const*) [/usr/local/Cellar/node/14.5.0/bin/node]
 4: 0x10018cc95 v8::Utils::ReportOOMFailure(v8::internal::Isolate*, char const*, bool) [/usr/local/Cellar/node/14.5.0/bin/node]
 5: 0x10018cc3f v8::internal::V8::FatalProcessOutOfMemory(v8::internal::Isolate*, char const*, bool) [/usr/local/Cellar/node/14.5.0/bin/node]
 6: 0x1002b2ff1 v8::internal::Heap::FatalProcessOutOfMemory(char const*) [/usr/local/Cellar/node/14.5.0/bin/node]
 7: 0x1002b4354 v8::internal::Heap::MarkCompactPrologue() [/usr/local/Cellar/node/14.5.0/bin/node]
 8: 0x1002b1d3b v8::internal::Heap::PerformGarbageCollection(v8::internal::GarbageCollector, v8::GCCallbackFlags) [/usr/local/Cellar/node/14.5.0/bin/node]
 9: 0x1002b04e4 v8::internal::Heap::CollectGarbage(v8::internal::AllocationSpace, v8::internal::GarbageCollectionReason, v8::GCCallbackFlags) [/usr/local/Cellar/node/14.5.0/bin/node]
10: 0x1002b8654 v8::internal::Heap::AllocateRawWithLightRetrySlowPath(int, v8::internal::AllocationType, v8::internal::AllocationOrigin, v8::internal::AllocationAlignment) [/usr/local/Cellar/node/14.5.0/bin/node]
11: 0x1002b86aa v8::internal::Heap::AllocateRawWithRetryOrFailSlowPath(int, v8::internal::AllocationType, v8::internal::AllocationOrigin, v8::internal::AllocationAlignment) [/usr/local/Cellar/node/14.5.0/bin/node]
12: 0x100294e11 v8::internal::FactoryBase<v8::internal::Factory>::NewRawOneByteString(int, v8::internal::AllocationType) [/usr/local/Cellar/node/14.5.0/bin/node]
13: 0x10036b2a7 v8::internal::JsonParser<unsigned char>::MakeString(v8::internal::JsonString const&, v8::internal::Handle<v8::internal::String>) [/usr/local/Cellar/node/14.5.0/bin/node]
14: 0x10036a2c1 v8::internal::JsonParser<unsigned char>::ParseJsonValue() [/usr/local/Cellar/node/14.5.0/bin/node]
15: 0x100369db8 v8::internal::JsonParser<unsigned char>::ParseJson() [/usr/local/Cellar/node/14.5.0/bin/node]
16: 0x100369d4d v8::internal::JsonParser<unsigned char>::Parse(v8::internal::Isolate*, v8::internal::Handle<v8::internal::String>, v8::internal::Handle<v8::internal::Object>) [/usr/local/Cellar/node/14.5.0/bin/node]
17: 0x10020bfa6 v8::internal::Builtin_Impl_JsonParse(v8::internal::BuiltinArguments, v8::internal::Isolate*) [/usr/local/Cellar/node/14.5.0/bin/node]
18: 0x10077d739 Builtins_CEntry_Return1_DontSaveFPRegs_ArgvOnStack_BuiltinExit [/usr/local/Cellar/node/14.5.0/bin/node]
19: 0x1ccf25a2d827
peterbe commented 4 years ago

@escattone Can you help figure out what's wrong? Using careful cut-and-rerun, I've figured out that if you remove

<p>{{page("/en-US/docs/Web/API/MediaSessionAction", "Syntax")}}</p>

then it works. So there's something going haywire inside that macros.

peterbe commented 4 years ago

If you put a console.log in src/api/wiki.js:

  //
  async page(path, section, revision, show, heading) {
+   console.log({path, section, revision, show, heading});
    // Adjusts the visibility and heading levels of the specified HTML.

...then your stdout goes nuts. Clearly there's some sort of infinite loop recursion going on.

I think what would be REALLY worthwhile and interesting is to first write some new code that counts the recursion depth and if it goes over a certain number, throws. I'd much rather get thrown errors than OOM crashes. Or, perhaps instead of a counter, we use a Set to see which prerequisites we've gone done into and if that repeats, throw an error.

peterbe commented 4 years ago

Oh I see. content/files/en-us/web/api/mediasessionactiondetails/index.html contains:

<p>{{page("/en-US/docs/Web/API/MediaSessionAction", "Syntax")}}</p>

and content/files/en-us/web/api/mediasessionaction/index.html contains:

<p>{{page("/en-US/docs/Web/API/MediaSessionActionDetails", "Examples")}}</p>

The reason it didn't fail in the WIki was presumably because it's able to re-use existing rendered HTML instead of our Yari JIT builder which will dig deep into every sub-rendering on the fly.

So, basically, how do we guard against such easy mistakes of authors creating an infinite loop?

peterbe commented 4 years ago

It's probably happening in web/api/rtcdtmftonechangeevent somewhere too. It's getting stuck somewhere there too. And those pages aren't using the {{page(...)}} macro.

peterbe commented 4 years ago

I think this page is another example: https://github.com/mdn/content/blob/0579292823efff25be2a7ec86e16adb63bc52e90/files/en-us/web/api/rtcerror/index.html#L54

peterbe commented 4 years ago

Note-to-selfs; http://localhost:3000/en-US/docs/Web/HTML/Element/input/tel uses the {{page()}} macro a lot. In good and sane ways. It's benefits from the memoization but if you disable memoization it should just get slower, not break.