Preserve the scope across the exception handler by vchuravy · Pull Request #60647 · JuliaLang/julia

vchuravy · 2026-01-12T18:41:40Z

Fixes
#60620 (comment)
previously we were preserving the new scope across the exception
handler, instead of the scope that was actually stored in the eh
context.

The additionaly write_barriers are probably not necessary (the task
should always be young.

Alternative to #60620

src/interpreter.c

src/codegen.cpp

Keno · 2026-01-12T18:47:09Z

LGTM on the root change, but NAK on the unnecessary write barriers as inline commented (and similar for the ones in the runtime).

src/task.c

src/interpreter.c

test/scopedvalues.jl

vchuravy · 2026-01-12T18:54:47Z

@Keno I added comments for the wbs. Does that look okay, or do you want to use something like jl_gc_wb_current_task?

Keno · 2026-01-12T18:57:06Z

@Keno I added comments for the wbs. Does that look okay, or do you want to use something like jl_gc_wb_current_task?

If you're gonna add a comment anyway, might as well make it executable, so that it's standardized.

Fixes JuliaLang#60620 (comment) previously we were preserving the new scope across the exception handler, instead of the scope that was actually stored in the eh context. The additionaly write_barriers are probably not necessary (the task should always be young. Alternative to JuliaLang#60620

vchuravy · 2026-01-12T20:01:26Z

If you're gonna add a comment anyway, might as well make it executable, so that it's standardized

I was looking for a previous example of using marker functions for this, but didn't find one. Do you have an example?

NHDaly · 2026-01-13T14:02:37Z

Why doesn't this one need to be backported to 1.11 and 1.13 as well?

vchuravy · 2026-01-13T15:49:06Z

Technically this needs to be back ported to v1.11 but I don't expect a new release there.

Why doesn't this one need to be backported to 1.11 and 1.13 as well?

Because I was looking at the wrong PR when I added the labels ;)

vchuravy · 2026-01-13T16:39:35Z

I wanted to add a LLVM code generation test, but looking at a slightly simplified version:

julia> using Base.ScopedValues: ScopedValue, with
julia> const sv = ScopedValue([])

julia> @noinline function inner()
           val = with(sv=>Any[2]) do
                   # Inside this new scope, when we perform GC, the parent scope can be freed.
                   # The fix for this issue made sure that the first scope in this task remains
                   # rooted.
                   GC.gc()
                   GC.gc()
                   sv[]
           end
           @show val
            # Finally, we've returned to the original scope, but that could be a dangling
               # pointer if the scope itself was freed by the above GCs. So these GCs could crash:
               GC.gc()
               GC.gc()
           end
julia> @noinline function outer()
           # Set a new Scope- this is the scope that could be freed.
               @inline with(sv=>Any[1]) do
                   return inner()
               end
           end

On 1.11:

julia> @code_llvm outer()
L52:                                              ; preds = %guard_exit16, %L25
   %value_phi = phi ptr [ %"box::Scope", %L25 ], [ %"box::Scope32", %guard_exit16 ]
   %current_scope = getelementptr inbounds ptr, ptr %tls_pgcstack52, i64 -9
   %9 = load ptr, ptr %current_scope, align 8
   store ptr %value_phi, ptr %current_scope, align 8
   %gc_slot_addr_0 = getelementptr inbounds ptr, ptr %gcframe1, i64 2
   store ptr %9, ptr %gc_slot_addr_0, align 16
   %10 = call i64 @ijl_excstack_state(ptr nonnull %current_task1)
   call void @llvm.lifetime.start.p0(i64 256, ptr nonnull %0)
   call void @ijl_enter_handler(ptr nonnull %current_task1, ptr nonnull %0)
   %11 = call i32 @__sigsetjmp(ptr nonnull %0, i32 0) #10
   %12 = icmp eq i32 %11, 0
   br i1 %12, label %try, label %catch_pop
try:                                              ; preds = %L52
; │┌ @ REPL[4]:4 within `#3`
    call void @j_inner_1330()
; │└
   call void @ijl_pop_handler_noexcept(ptr nonnull %current_task1, i32 1)
   call void @llvm.lifetime.end.p0(i64 256, ptr nonnull %0)
   store ptr %9, ptr %current_scope, align 8
   %frame.prev51 = load ptr, ptr %frame.prev, align 8
   store ptr %frame.prev51, ptr %tls_pgcstack52, align 8
   ret void

catch_pop:                                        ; preds = %L52
   call void @ijl_pop_handler(ptr nonnull %current_task1, i32 1)
   call void @llvm.lifetime.end.p0(i64 256, ptr nonnull %0)
   store ptr %9, ptr %current_scope, align 8
   call void @j_rethrow_1332() #8
   unreachable

So %9 contains the old scope, and it is rooted through the gc_frame. We set the new scope before we call jl_enter_handler
So in 1.11 this seems to be all correct. And indeed reading through #52309 there we do indeed save the old scope correctly.

It is only in #55907 when the scope is moved into the _eh_frame that we are preserving the "wrong one", and indeed

L51:                                              ; preds = %guard_exit, %L24
   %value_phi = phi ptr [ %"box::Scope", %L24 ], [ %"box::Scope30", %guard_exit ]
   %gc_slot_addr_0 = getelementptr inbounds ptr, ptr %gcframe1, i64 2
   store ptr %value_phi, ptr %gc_slot_addr_0, align 8
   %6 = call i64 @ijl_excstack_state(ptr nonnull %current_task)
   call void @llvm.lifetime.start.p0(i64 264, ptr nonnull %0)
   call void @ijl_enter_handler(ptr nonnull %current_task, ptr nonnull %0)
   %7 = call i32 @__sigsetjmp(ptr nonnull %0, i32 0) #8
   %8 = icmp eq i32 %7, 0
   br i1 %8, label %try, label %catch_pop

So we don't need to backport this to 1.11

Fixes #60620 (comment) previously we were preserving the new scope across the exception handler, instead of the scope that was actually stored in the eh context. The additionaly write_barriers are probably not necessary (the task should always be young. Alternative to #60620

src/gc-interface.h

src/task.c

Co-authored-by: Keno Fischer <keno@juliacomputing.com>

NHDaly · 2026-01-15T05:09:07Z

Thanks again Valentin! 🎉

NHDaly · 2026-01-15T05:09:08Z

Thanks again Valentin! 🎉

We store the previous scope in the eh_state and thus hide it from GC. This means we need to manually preserve that scope across the `try ... catch`, instead fo the new scope that we switch to. --------- Co-authored-by: Nathan Daly <nathan.daly@relational.ai> Co-authored-by: Keno Fischer <keno@juliacomputing.com>

* Preserve the scope across the exception handler (JuliaLang#60647) We store the previous scope in the eh_state and thus hide it from GC. This means we need to manually preserve that scope across the `try ... catch`, instead fo the new scope that we switch to. --------- Co-authored-by: Nathan Daly <nathan.daly@relational.ai> Co-authored-by: Keno Fischer <keno@juliacomputing.com> * add wb_back on all task switch paths (JuliaLang#60617) Since this task's stack or scope field could have been modified after it was marked by an incremental collection (and not just for copy stacks), move the barrier back unconditionally here. --------- Co-authored-by: Valentin Churavy <v.churavy@gmail.com> Co-authored-by: Jeff Bezanson <jeff.bezanson@gmail.com> --------- Co-authored-by: Valentin Churavy <v.churavy@gmail.com> Co-authored-by: Keno Fischer <keno@juliacomputing.com> Co-authored-by: Jameson Nash <vtjnash@gmail.com> Co-authored-by: Jeff Bezanson <jeff.bezanson@gmail.com>

We store the previous scope in the eh_state and thus hide it from GC. This means we need to manually preserve that scope across the `try ... catch`, instead fo the new scope that we switch to. --------- Co-authored-by: Nathan Daly <nathan.daly@relational.ai> Co-authored-by: Keno Fischer <keno@juliacomputing.com> (cherry picked from commit f22ae77)

DilumAluthge · 2026-01-19T17:09:07Z

[This doesn't backport cleanly to 1.12. I've asked Valentin if he can put together a manual backport to backports-release-1.12.]

We store the previous scope in the eh_state and thus hide it from GC. This means we need to manually preserve that scope across the `try ... catch`, instead fo the new scope that we switch to. --------- Co-authored-by: Nathan Daly <nathan.daly@relational.ai> Co-authored-by: Keno Fischer <keno@juliacomputing.com> (cherry picked from commit f22ae77)

* Preserve the scope across the exception handler (JuliaLang#60647) We store the previous scope in the eh_state and thus hide it from GC. This means we need to manually preserve that scope across the `try ... catch`, instead fo the new scope that we switch to. --------- Co-authored-by: Nathan Daly <nathan.daly@relational.ai> Co-authored-by: Keno Fischer <keno@juliacomputing.com> * add wb_back on all task switch paths (JuliaLang#60617) Since this task's stack or scope field could have been modified after it was marked by an incremental collection (and not just for copy stacks), move the barrier back unconditionally here. --------- Co-authored-by: Valentin Churavy <v.churavy@gmail.com> Co-authored-by: Jeff Bezanson <jeff.bezanson@gmail.com> --------- Co-authored-by: Valentin Churavy <v.churavy@gmail.com> Co-authored-by: Keno Fischer <keno@juliacomputing.com> Co-authored-by: Jameson Nash <vtjnash@gmail.com> Co-authored-by: Jeff Bezanson <jeff.bezanson@gmail.com>

We store the previous scope in the eh_state and thus hide it from GC. This means we need to manually preserve that scope across the `try ... catch`, instead fo the new scope that we switch to. --------- Co-authored-by: Nathan Daly <nathan.daly@relational.ai> Co-authored-by: Keno Fischer <keno@juliacomputing.com> (cherry picked from commit f22ae77)

* Preserve the scope across the exception handler (JuliaLang#60647) We store the previous scope in the eh_state and thus hide it from GC. This means we need to manually preserve that scope across the `try ... catch`, instead fo the new scope that we switch to. --------- Co-authored-by: Nathan Daly <nathan.daly@relational.ai> Co-authored-by: Keno Fischer <keno@juliacomputing.com> * add wb_back on all task switch paths (JuliaLang#60617) Since this task's stack or scope field could have been modified after it was marked by an incremental collection (and not just for copy stacks), move the barrier back unconditionally here. --------- Co-authored-by: Valentin Churavy <v.churavy@gmail.com> Co-authored-by: Jeff Bezanson <jeff.bezanson@gmail.com> --------- Co-authored-by: Valentin Churavy <v.churavy@gmail.com> Co-authored-by: Keno Fischer <keno@juliacomputing.com> Co-authored-by: Jameson Nash <vtjnash@gmail.com> Co-authored-by: Jeff Bezanson <jeff.bezanson@gmail.com>

vchuravy mentioned this pull request Jan 12, 2026

GC-preserve a Task's scope on start, to prevent freeing unrooted scopes via ScopedThunk #60620

Closed

vchuravy requested review from Keno and topolarity January 12, 2026 18:43

NHDaly reviewed Jan 12, 2026

View reviewed changes

src/interpreter.c Outdated Show resolved Hide resolved

NHDaly reviewed Jan 12, 2026

View reviewed changes

src/codegen.cpp Outdated Show resolved Hide resolved

Keno reviewed Jan 12, 2026

View reviewed changes

src/codegen.cpp Outdated Show resolved Hide resolved

Keno reviewed Jan 12, 2026

View reviewed changes

src/task.c Outdated Show resolved Hide resolved

NHDaly approved these changes Jan 12, 2026

View reviewed changes

Keno reviewed Jan 12, 2026

View reviewed changes

src/interpreter.c Outdated Show resolved Hide resolved

NHDaly reviewed Jan 12, 2026

View reviewed changes

test/scopedvalues.jl Show resolved Hide resolved

NHDaly mentioned this pull request Jan 12, 2026

Fix GC corruption via two upstream PRs. RelationalAI/julia#272

Merged

3 tasks

vchuravy requested a review from d-netto as a code owner January 13, 2026 10:57

vchuravy added the backport 1.12 Change should be backported to release-1.12 label Jan 13, 2026

vchuravy added backport 1.11 Change should be backported to release-1.11 backport 1.13 Change should be backported to release-1.13 labels Jan 13, 2026

vchuravy removed the backport 1.11 Change should be backported to release-1.11 label Jan 13, 2026

vchuravy and others added 5 commits January 13, 2026 17:45

Add test that Tasks are correctly preserved even across Task spawns

2245ddb

drop write_barriers

ee394b0

fixup comments

279a360

use marker function

1b67f8f

vchuravy force-pushed the vc/preserve_old_scope branch from 37d2b0d to 1b67f8f Compare January 13, 2026 16:45

Keno reviewed Jan 13, 2026

View reviewed changes

src/gc-interface.h Outdated Show resolved Hide resolved

Keno reviewed Jan 13, 2026

View reviewed changes

src/task.c Outdated Show resolved Hide resolved

vchuravy and others added 2 commits January 13, 2026 19:17

Update src/gc-interface.h

bceec99

Co-authored-by: Keno Fischer <keno@juliacomputing.com>

use knownold

f7ce7a4

vchuravy merged commit f22ae77 into master Jan 14, 2026
8 checks passed

vchuravy deleted the vc/preserve_old_scope branch January 14, 2026 13:40

DilumAluthge mentioned this pull request Jan 15, 2026

Backports for 1.12.5 #60612

Merged

40 tasks

kpamnany mentioned this pull request Jan 20, 2026

GC error on Julia 1.12 #59483

Closed

KristofferC mentioned this pull request Jan 26, 2026

Backports for 1.13.0-beta2 #60614

Merged

43 tasks

KristofferC removed backport 1.12 Change should be backported to release-1.12 backport 1.13 Change should be backported to release-1.13 labels Feb 3, 2026

Uh oh!

Conversation

vchuravy commented Jan 12, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Keno commented Jan 12, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vchuravy commented Jan 12, 2026

Uh oh!

Keno commented Jan 12, 2026

Uh oh!

vchuravy commented Jan 12, 2026

Uh oh!

NHDaly commented Jan 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vchuravy commented Jan 13, 2026

Uh oh!

vchuravy commented Jan 13, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

NHDaly commented Jan 15, 2026

Uh oh!

NHDaly commented Jan 15, 2026

Uh oh!

DilumAluthge commented Jan 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

NHDaly commented Jan 13, 2026 •

edited

Loading