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

Temporary fix for the Julia GC. #3412

Merged
merged 4 commits into from
Apr 25, 2019

Conversation

rbehrends
Copy link
Contributor

This addresses a problem where the Julia GC during a partial sweep frees
some, but not all objects of an unreachable data structure. If an
address to a remaining object of such a data structure is still randomly
hit during conservative stack scanning, it may erroneously try to mark
the deallocated objects, too.

This temporary fix works by validating each pointer before it is marked.
Because this is potentially expensive, the eventual solution should fix
the root cause, which is conservative stack scanning resurrecting
pointers to dead objects that have been freed during a partial
collection.

@coveralls
Copy link

coveralls commented Apr 15, 2019

Coverage Status

Coverage decreased (-0.007%) to 85.222% when pulling 49f65b4 on rbehrends:julia-gc-validate-strict into 9886081 on gap-system:master.

@fingolfin fingolfin added kind: bug Issues describing general bugs, and PRs fixing them kind: bug: crash Issues describing bugs that cause GAP to crash, and PRs fixing them (used for release notes) topic: julia Julia GC integration and related matters topic: kernel labels Apr 15, 2019
src/julia_gc.c Outdated Show resolved Hide resolved
src/julia_gc.c Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Apr 16, 2019

Codecov Report

Merging #3412 into master will increase coverage by 0.01%.
The diff coverage is 66.66%.

@@            Coverage Diff             @@
##           master    #3412      +/-   ##
==========================================
+ Coverage    85.4%   85.41%   +0.01%     
==========================================
  Files         698      697       -1     
  Lines      344337   342390    -1947     
==========================================
- Hits       294097   292469    -1628     
+ Misses      50240    49921     -319
Impacted Files Coverage Δ
src/weakptr.c 97.89% <100%> (-0.03%) ⬇️
src/julia_gc.c 77.4% <57.69%> (-9.79%) ⬇️
src/modules.h 33.33% <0%> (-66.67%) ⬇️
src/cyclotom.h 50% <0%> (-50%) ⬇️
src/records.h 50% <0%> (-50%) ⬇️
src/gapstate.h 42.85% <0%> (-42.86%) ⬇️
src/vec8bit.h 51.78% <0%> (-42.86%) ⬇️
src/macfloat.h 57.14% <0%> (-42.86%) ⬇️
src/code.h 57.14% <0%> (-40.16%) ⬇️
src/gvars.h 66.66% <0%> (-33.34%) ⬇️
... and 90 more

src/julia_gc.c Outdated Show resolved Hide resolved
src/julia_gc.c Outdated Show resolved Hide resolved
src/julia_gc.c Show resolved Hide resolved
src/julia_gc.c Show resolved Hide resolved
src/julia_gc.c Outdated Show resolved Hide resolved
src/julia_gc.c Outdated Show resolved Hide resolved
src/julia_gc.c Outdated Show resolved Hide resolved
src/julia_gc.c Show resolved Hide resolved
src/julia_gc.c Show resolved Hide resolved
src/julia_gc.c Show resolved Hide resolved
src/julia_gc.c Outdated Show resolved Hide resolved
src/julia_gc.c Outdated Show resolved Hide resolved
src/julia_gc.c Show resolved Hide resolved
src/julia_gc.c Outdated Show resolved Hide resolved
src/julia_gc.h Outdated Show resolved Hide resolved
src/julia_gc.h Outdated Show resolved Hide resolved
src/julia_gc.h Outdated Show resolved Hide resolved
src/julia_gc.h Outdated Show resolved Hide resolved
@rbehrends rbehrends force-pushed the julia-gc-validate-strict branch 4 times, most recently from 16169ca to 56750a5 Compare April 23, 2019 09:49
@fingolfin fingolfin added this to the GAP 4.11.0 milestone Apr 23, 2019
src/julia_gc.c Outdated Show resolved Hide resolved
src/julia_gc.h Outdated Show resolved Hide resolved
src/julia_gc.h Outdated Show resolved Hide resolved
src/julia_gc.h Outdated Show resolved Hide resolved
src/julia_gc.c Show resolved Hide resolved
src/julia_gc.c Outdated Show resolved Hide resolved
@rbehrends rbehrends force-pushed the julia-gc-validate-strict branch 3 times, most recently from efbb767 to 6ab4559 Compare April 24, 2019 12:11
This addresses a problem where the Julia GC during a partial sweep frees
some, but not all objects of an unreachable data structure. If an
address to a remaining object of such a data structure is still randomly
hit during conservative stack scanning, it may erroneously try to mark
the deallocated objects, too.

This temporary fix works by validating each pointer before it is marked.
Because this is potentially expensive, the eventual solution should fix
the root cause, which is conservative stack scanning resurrecting
pointers to dead objects that have been freed during a partial
collection.
@fingolfin fingolfin added the release notes: to be added PRs introducing changes that should be (but have not yet been) mentioned in the release notes label Apr 25, 2019
@fingolfin fingolfin merged commit 5d3e72a into gap-system:master Apr 25, 2019
@fingolfin fingolfin deleted the julia-gc-validate-strict branch June 4, 2019 11:53
@fingolfin fingolfin mentioned this pull request Jun 4, 2019
@fingolfin fingolfin modified the milestone: GAP 4.10.2 Jun 13, 2019
@olexandr-konovalov olexandr-konovalov added release notes: added PRs introducing changes that have since been mentioned in the release notes and removed release notes: to be added PRs introducing changes that should be (but have not yet been) mentioned in the release notes labels Jun 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-to-4.10-DONE kind: bug: crash Issues describing bugs that cause GAP to crash, and PRs fixing them (used for release notes) kind: bug Issues describing general bugs, and PRs fixing them release notes: added PRs introducing changes that have since been mentioned in the release notes topic: julia Julia GC integration and related matters topic: kernel
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants