[release-1.10] dont reset maxsize in jl_array_to_string

[d-netto]

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> 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> 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

[oscardssmith]

The issue with this is that it then becomes UB to call jl_array_to_string and then push! to the vector

[d-netto]

The issue with this is that it then becomes UB to call jl_array_to_string and then push! to the vector

Not sure if I see why?

The return jl_pchar_to_string((const char*)jl_array_data(a), len); statement at the end of the function is allocating a new buffer for the String either way.

[d-netto]

Bump.

[NHDaly]

Yeah, @oscardssmith I don't think you have that quite right.
This code is pretty tricky, and took us quite some time to wrap our heads around it. It is surely lacking in comments. (We would fix that, but the whole thing is deleted after the memory PR.)

Is there any chance that you misread things? Can you give it a closer read?

As diogo says: in the branch this PR modifies, the data is not being shared, it's being copied. The only reason this branch resets the values in the array is to be consistent with the branch where it is shared. So after this change, that branch will logically clear the array, in all user-visible ways, but it will allow the array to keep accounting for its memory, so that the memory accounting is correct.

[NHDaly]

This LGTM. I'm pretty sure this is the right fix, and it resolved the issues from our end.

@oscardssmith if you could give it another look that would be helpful, but i have medium-high confidence in this approach and I'm comfortable approving+merging.