Skip to content

Tab Crashing Fixes#203

Merged
tmbrbr merged 3 commits into
SAP:mainfrom
tmbrbr:tabcrash-fixes
Feb 23, 2024
Merged

Tab Crashing Fixes#203
tmbrbr merged 3 commits into
SAP:mainfrom
tmbrbr:tabcrash-fixes

Conversation

@tmbrbr

@tmbrbr tmbrbr commented Feb 23, 2024

Copy link
Copy Markdown
Contributor

Fixing a few bugs introduced by recent changes:

  • Memory allocation issue in JSONParser - the CurrentJsonPath method was creating a lot of Strings in the nursery, which I think was overwhelming the GC at some point. Solution is to create the strings in the Tenured heap.
  • Memory leak caused by creation of raw StringTaint objects (which do not clean up when destroyed)

I also found the dump() method which was handy to debug the JSONParser issue. I've added a TAINT_DEBUG check to it so it can be enabled which needed a complete debug build.

The CurrentJsonPath method was causing tab crashes when
parsing large JSON files. These seem to occur when the JSON
parser was running at the same time as Nursery Strings were
moved to Tenured space. My feeling is that the JSONPath strings
were being created faster than they could be moved out of the
nursery, causing some nasty segfaults.

A quick for this appears to be to create the JSONPath strings
directly in the Tenured heap. This is probably not ideal, as
these strings are only used to create TaintOperations.

This is probably worth revisiting to see if there is a better
solution. There seems to be a lot of upcoming upstream changes
related to String GC, so probably worth waiting for the next
release (see e.g. 7e2b43b)
@tmbrbr tmbrbr self-assigned this Feb 23, 2024
@tmbrbr tmbrbr added bug Something isn't working javascript Pull requests that update Javascript code performance Performance Issues and Enhancements labels Feb 23, 2024
@tmbrbr tmbrbr requested review from leeN and vladidx February 23, 2024 09:21
@tmbrbr

tmbrbr commented Feb 23, 2024

Copy link
Copy Markdown
Contributor Author

@leeN: we should probably use this fix for any crawls to increase stability and stop horrendous memory leaks!

@vladidx: this should resolve the stability issues we discussed.

@tmbrbr tmbrbr changed the title Tabcrash fixes Tab Crashing Fixes Feb 23, 2024

@leeN leeN left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks good to me!

@tmbrbr tmbrbr merged commit d85187f into SAP:main Feb 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working javascript Pull requests that update Javascript code performance Performance Issues and Enhancements

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants