Skip to content

Conversation

@stesie
Copy link
Member

@stesie stesie commented Aug 1, 2015

  • fix module caching to use string comparison (instead of std::map default behaviour of pointer comparison)
  • fixes several use-after-free issues
  • fixes several memory leaks
  • increases test coverage on v8js_commonjs.cc to 100%

After all this might be bit of a backwards incompatible change, that caching is now applied correctly (and some code might rely on caching not to work). Contrary without this patch chances are that two distinctly named modules are collapsed & cached, if both used the exact same pointer (which could happen due to a use-after-free issue)

@pinepain would you like to give it a try?

This fixes #140

stesie added 10 commits August 1, 2015 17:09
Without the compare function std::map simply compares one
pointer to another.  However we need to compare the actual
strings.

Besides we must not efree normalised_module_id on return
of require method, since the pointer was added to
modules_loaded (and is valid until destruction of V8Js
class); instead it is now freed during V8Js object destruction.
The garbage collector cannot free the object as it is allocated
on the local stack.
The strlen usage on term obviously was wrong here, since the term
string is not null-terminated at that place.
stesie added a commit that referenced this pull request Aug 3, 2015
Fix module caching & memory leaks
@stesie stesie merged commit 829bac9 into phpv8:master Aug 3, 2015
@stesie stesie deleted the issue-140 branch December 31, 2015 20:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant