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

Fix memory leak when using continue or break statement with syntaxError #5190

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

gergocs
Copy link
Contributor

@gergocs gergocs commented Dec 9, 2024

This patch fixes #5062.

JerryScript-DCO-1.0-Signed-off-by: Gergo Csizi gergocs@inf.u-szeged.hu

@gergocs gergocs force-pushed the fixIssue5062 branch 2 times, most recently from 11547fa to e37002c Compare December 9, 2024 09:58
@gergocs gergocs marked this pull request as ready for review December 9, 2024 09:58
Copy link
Member

@zherczeg zherczeg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Title: whit -> with

jerry-core/parser/js/js-parser-statm.c Outdated Show resolved Hide resolved
@gergocs gergocs changed the title Fix memory leak when using continue or break statement whit syntaxError Fix memory leak when using continue or break statement with syntaxError Dec 9, 2024
@gergocs gergocs requested a review from zherczeg December 9, 2024 12:21
@zherczeg
Copy link
Member

zherczeg commented Dec 9, 2024

Does this patch increase memory consumption in the normal (no parse error) case?

@gergocs
Copy link
Contributor Author

gergocs commented Dec 9, 2024

Yes it does. For example the regression-test-issue-1555.js test fails if the heap size is not increased.

@zherczeg
Copy link
Member

zherczeg commented Dec 9, 2024

Would it be possible to free the memory at its regular place as well?

@gergocs
Copy link
Contributor Author

gergocs commented Dec 9, 2024

I don't think so. I have debugging this issue a very long time and didn't find any better methods to keep track of possible non freed pointers.

@zherczeg
Copy link
Member

zherczeg commented Dec 9, 2024

I mean when a pointer is not needed, it could be removed from the tracker and free it. I suspect we keep pointers too long around this way.

@gergocs gergocs force-pushed the fixIssue5062 branch 2 times, most recently from 0bf29cb to 66c17dd Compare December 11, 2024 07:52
This patch fixes jerryscript-project#5062.

JerryScript-DCO-1.0-Signed-off-by: Gergo Csizi gergocs@inf.u-szeged.hu
@zherczeg
Copy link
Member

parser_raise_error (parser_context_t *context_p, /**< context */

The parser already does a lot of cleanup on throw. If we could put this branch list into some local context (a chain list with a head), we could cleanup it without complex structures.

@gergocs
Copy link
Contributor Author

gergocs commented Dec 11, 2024

Currently it is a chain list not a complex structure and stored in the context to easily pass around.

@zherczeg
Copy link
Member

Not the complexity, but it has while loops. The design is better if less scanning is needed.

Is it operating like a stack btw? So we usually remove items form the end?

@gergocs
Copy link
Contributor Author

gergocs commented Dec 11, 2024

No, it's not like stack.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants