#3193 closed defect (fixed)
TplParser - stack overflow for big templates
Reported by: | adrpo | Owned by: | sjoelund.se |
---|---|---|---|
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 Changed 10 years ago by adrpo
comment:2 Changed 10 years ago by sjoelund.se
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 Changed 10 years ago by adrpo
I inlined TplParser.templateBody inside TplParser.restOfTemplLine but TplParser.restOfTemplLine used matchcontinue.
comment:4 Changed 10 years ago by adrpo
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 Changed 10 years ago by sjoelund.se
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 Changed 10 years ago by adrpo
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 Changed 10 years ago by perost
- Resolution set to fixed
- Status changed from new to closed
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 Changed 10 years ago by adrpo
Thnks! I now removed the ulimit -s from the Linux nightly builds.
comment:9 Changed 9 years ago by dietmarw
- Milestone changed from Future to pre1.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 Changed 7 years ago by sjoelund.se
- Milestone changed from pre1.9.4 to 1.9.4
Removing the pre1.9.4 milestone in favor of 1.9.4.
Seems that ulimit -s 14336, 14MB works but not 13MB.