Data race between Array.chop and Array.undefined's use of realloc

[jasoncarr0]

This code presented by @ergl in Zulip demonstrates a data race where the update to change the last character to '?' can be observed in the ArrayGrower actor within undefined.

actor Main
  new create(env: Env) =>
    let arr = "hello, world!".clone()
    (let hello, let rest) = (consume arr).chop(5)

    ArrayGrower(env, consume hello)
    Modifier(consume rest)

actor Modifier
  be apply(data: String iso) =>
    try data(data.size()-1)? = '?' end

actor ArrayGrower
  be apply(env: Env, data: String iso) =>
    let arr = (consume data).iso_array()
    arr.undefined(13)
    let final = String.from_iso_array(consume arr)
    env.out.print(consume final)

This occurs because Array.undefined calls the pony_realloc runtime function. The implementation at ponyint_heap_realloc then computes the size using the chunk info https://github.com/ponylang/ponyc/blob/935b9136a56685199b1602c51607bdcbdc92caaf/src/libponyrt/mem/heap.c#L571 and copies all of the old data over from the old chunk to the new one.

Unfortunately, if the realloced chunk comes from a sub-Array that comes from Array.chop(), then the length of the chunk will reflect the entire allocation, instead of length of the sub-Array (which grants the capability to read).

This causes realloc to copy the entire allocation, including the portion from the other half of the chop, creating a data race between realloc and any writes that might occur in another actor, with both actions unsynchronized.

[jasoncarr0]

A likely fix for this would be adding a length parameter to realloc to state how much data should be copied, or to not copy any data within the realloc function, and instead copy the data within pony code. But in either case, all uses of realloc would need to be changed.

[SeanTAllen]

With the realloc change, attention with need to be taken to make sure gc tracking is correct.

[SeanTAllen]

@jemc do you have any thoughts on this?

[SeanTAllen]

before anyone tackles fixing this, please consult with at least @jemc and myself to get sign off on your approach. @jasoncarr0 has suggested a couple possible approaches.

[SeanTAllen]

Note because this is a race, you will only sometimes see the change from ! to ?

[SeanTAllen]

I have this fixed on a branch but it needs a test and release notes.