Skip to content

Commit

Permalink
[release-1.10] dont reset maxsize in jl_array_to_string (#55689)
Browse files Browse the repository at this point in the history
Let's change `jl_array_to_string` so that we make the consequences of
calling it in a thread-unsafe way less disastrous (i.e. let's avoid
corrupting GC internal metrics).

Strictly speaking, this is not a bug-fix because calling
`jl_array_to_string` concurrently from two threads is UB in any case.

To see how a race here may lead to negative `live_bytes`, consider this
MWE from @NHDaly:

- 1.10:
```Julia
julia> GC.gc(true); Base.gc_live_bytes()
1842370

julia> g_vecs = Any[
                  UInt8['a' for _ in 1:1000000000]
                  for _ in 1:10
                  ];

julia> GC.gc(true); Base.gc_live_bytes()
10001774906

julia> Threads.@threads for _ in 1:1000
                  for v in g_vecs
                      String(v)
                  end
              end

julia> GC.gc(true); Base.gc_live_bytes()
-1997600207
```

- This patch:
```Julia
julia> GC.gc(true); Base.gc_live_bytes()
1862440

julia> g_vecs = Any[
                  UInt8['a' for _ in 1:1000000000]
                  for _ in 1:10
                  ];

julia> GC.gc(true); Base.gc_live_bytes()
10001796440

julia> Threads.@threads for _ in 1:1000
                  for v in g_vecs
                      String(v)
                  end
              end

julia> GC.gc(true); Base.gc_live_bytes()
10002390952
```
  • Loading branch information
d-netto authored Sep 12, 2024
1 parent c7f5f2d commit c5ab7c9
Show file tree
Hide file tree
Showing 3 changed files with 0 additions and 10 deletions.
2 changes: 0 additions & 2 deletions src/array.c
Original file line number Diff line number Diff line change
Expand Up @@ -479,10 +479,8 @@ JL_DLLEXPORT jl_value_t *jl_array_to_string(jl_array_t *a)
return o;
}
}
jl_gc_count_freed(jl_array_nbytes(a));
a->nrows = 0;
a->length = 0;
a->maxsize = 0;
return jl_pchar_to_string((const char*)jl_array_data(a), len);
}

Expand Down
7 changes: 0 additions & 7 deletions src/gc.c
Original file line number Diff line number Diff line change
Expand Up @@ -1128,13 +1128,6 @@ void jl_gc_count_allocd(size_t sz) JL_NOTSAFEPOINT
jl_atomic_load_relaxed(&ptls->gc_num.allocd) + sz);
}

void jl_gc_count_freed(size_t sz) JL_NOTSAFEPOINT
{
jl_ptls_t ptls = jl_current_task->ptls;
jl_atomic_store_relaxed(&ptls->gc_num.freed,
jl_atomic_load_relaxed(&ptls->gc_num.freed) + sz);
}

static void combine_thread_gc_counts(jl_gc_num_t *dest) JL_NOTSAFEPOINT
{
int gc_n_threads;
Expand Down
1 change: 0 additions & 1 deletion src/julia_internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -573,7 +573,6 @@ JL_DLLEXPORT int64_t jl_gc_sync_total_bytes(int64_t offset) JL_NOTSAFEPOINT;
void jl_gc_track_malloced_array(jl_ptls_t ptls, jl_array_t *a) JL_NOTSAFEPOINT;
size_t jl_array_nbytes(jl_array_t *a) JL_NOTSAFEPOINT;
void jl_gc_count_allocd(size_t sz) JL_NOTSAFEPOINT;
void jl_gc_count_freed(size_t sz) JL_NOTSAFEPOINT;
void jl_gc_run_all_finalizers(jl_task_t *ct);
void jl_release_task_stack(jl_ptls_t ptls, jl_task_t *task);
void jl_gc_add_finalizer_(jl_ptls_t ptls, void *v, void *f) JL_NOTSAFEPOINT;
Expand Down

0 comments on commit c5ab7c9

Please sign in to comment.