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

SourceExprCommand: allocate the vSourceExpr via uncollectable memory #3492

Merged
merged 1 commit into from Apr 15, 2020

Conversation

andir
Copy link
Member

@andir andir commented Apr 13, 2020

Previously the memory would occasionally be collected during eval since
the GC doesn't consider the member variable as alive / doesn't scan the
region of memory where the pointer lives.

By using the traceable_allocator allocator provided by Boehm GC we
can ensure the memory isn't collected. It should be properly freed when
SourceExprCommand goes out of scope.

Related to #3475

I've been running my reproducer for >20min. Usually it would trigger the error within a few minutes. So far it looks very stable. Having spent some time on reading through the code and the various GC docs I believe this might be the correct fix (or approach).

The vSourceExpr will be released when the current command (CmdBulid) goes out of scope. That is probably fine as that is basically the same as the lifetime of the entire program.

I would appreciate if we could get this into a stable (v2.3-branch) release.

Previously the memory would occasionally be collected during eval since
the GC doesn't consider the member variable as alive / doesn't scan the
region of memory where the pointer lives.

By using the traceable_allocator<T> allocator provided by Boehm GC we
can ensure the memory isn't collected. It should be properly freed when
SourceExprCommand goes out of scope.
@nagisa
Copy link

nagisa commented Jun 22, 2020

Can this be included into next point release?

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.

None yet

3 participants