Opened 10 years ago

Closed 10 years ago

Last modified 7 years ago

#3193 closed defect (fixed)

TplParser - stack overflow for big templates

Reported by: Adrian Pop Owned by: Martin Sjölund
Priority: high Milestone: 1.9.4
Component: Parser Version: trunk
Keywords: Cc:

Description

Seems that TplParser stack overflows for big templates via mutually recursive
TplParser.templateBody -> TplParser.restOfTemplLine -> TplParser.templateBody
See:
https://test.openmodelica.org/hudson/job/OpenModelica_LINUX_NIGHTLY_BUILD/1040/console

It seems that a rewrite is needed.

Change History (10)

comment:1 by Adrian Pop, 10 years ago

Seems that ulimit -s 14336, 14MB works but not 13MB.

comment:2 by Martin Sjölund, 10 years ago

I think TplParser.restOfTemplLine can be rewritten using guards and get something tail recursive. I had planned on rewriting this, but we had rml-mmc at the time.

comment:3 by Adrian Pop, 10 years ago

I inlined TplParser.templateBody inside TplParser.restOfTemplLine but TplParser.restOfTemplLine used matchcontinue.

Version 0, edited 10 years ago by Adrian Pop (next)

comment:4 by Adrian Pop, 10 years ago

The last 5 cases in TplParser.restOfTemplLine can now be merged into one case with a matchcontinue and match could be used at top-level but i don't know if this would help.

comment:5 by Martin Sjölund, 10 years ago

All of the cases in restOfTempLine can be merged into a single match with guards.
Note that the 5th last case calls newLine, which just checks if the next character is CR or LF and does some stuff. So you can simply make a guard isNewline.

The first cases just check if the first character is lesc, which is easy to guard (guard c==lesc).

comment:6 by Adrian Pop, 10 years ago

Well, I'm not thinking very new MetaModelica yet but what you say seems possible :)
For the Linux builds I put ulimit -s 16384 inside: /home/build/apt-build/make-chroot-package.sh for now and it seems to work but I guess we need to fix it properly.

comment:7 by Per Östlund, 10 years ago

Resolution: fixed
Status: newclosed

It seems I missed this ticket, I thought I was the only one with this issue and was already working on a fix. It should be fixed now in r24978.

comment:8 by Adrian Pop, 10 years ago

Thnks! I now removed the ulimit -s from the Linux nightly builds.

comment:9 by Dietmar Winkler, 9 years ago

Milestone: Futurepre1.9.4

It doesn't make sense to keep closed ticket in the "Future" milestone that were simply forgotten to assign to the correct milestone in the past.

comment:10 by Martin Sjölund, 7 years ago

Milestone: pre1.9.41.9.4

Removing the pre1.9.4 milestone in favor of 1.9.4.

Note: See TracTickets for help on using tickets.