Skip to content

Conversation

@ThomsonTan
Copy link
Collaborator

CapturedValues with 2 members of SListBase is used as stack variable in GlobOpt. The dtor of SListBase will complain if exception happened before destructing CapturedValues. This fix added dtor to CapturedValues to Reset 2 SListBase member before destruct them.

@ThomsonTan
Copy link
Collaborator Author

@curtisman could you please take a look?

@rajatd
Copy link
Contributor

rajatd commented Aug 5, 2016

:shipit:

{
// Reset SListBase to be exception safe. Captured values are from GlobOpt->func->alloc
// in normal case the 2 SListBase are empty so no Clear needed, also no need to Clear in exception case
constantValues.Reset();
Copy link
Contributor

Choose a reason for hiding this comment

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

Use clear instead. Especially if you are not using a local arena, but the Func arena, so the memorywill not be freed after Globopt. and be wasted until the Func finish JIT'ing.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@curtisman , in normal cases, these 2 local SListBase would have 0 elements because it just host the values temporarily, all the values are moved to bailoutInfo at the end of value capture which will be cleared at BailOutInfo::Clear(). The only case is when exception happened and the nodes have not been moved to bailoutInfo. But in this case the function will abort compilation so no need to free nodes to func->alloc aggresively. This idea looks similar to SListBase, the code who uses it need to ensure memory are freed, not the SListBase itself (dtor).

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds good.

@ThomsonTan
Copy link
Collaborator Author

@dotnet-bot test this please.

@curtisman
Copy link
Contributor

:shipit:

@chakrabot chakrabot merged commit d4d8e2b into chakra-core:master Aug 8, 2016
chakrabot pushed a commit that referenced this pull request Aug 8, 2016
…stBase members to be exception safe

Merge pull request #1395 from ThomsonTan:FixCapturedValuesException

CapturedValues with 2 members of SListBase is used as stack variable in GlobOpt. The dtor of SListBase will complain if exception happened before destructing CapturedValues.  This fix added dtor to CapturedValues to Reset 2 SListBase member before destruct them.
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.

5 participants