Race condition in pipe selecting

[eholk]

It's frustratingly nondeterministic, but we've been seeing it in the bots the past few days. See http://bot.rust-lang.org/logs/2012/07/03/2012-08-03T15:56:21Z-f86aabed-660e-45c0-bfa8-22930047515d.html for an example of what it looks like. (I would have pasted it here, but copy and paste was messing up the formatting).

It looks like we basically end up signaling an event on a task that has already been destroyed.

[eholk]

bd19551 should have removed one case where this race can happen, but apparently there are more.

[eholk]

Here's another instance of it: http://bot.rust-lang.org/logs/2012/07/03/2012-08-03T17:36:59Z-77010905-0957-45df-b460-9999ce0c64f1.html

[bblum]

one approach might be to add runtime calls to modify a task's atomic refcount - reference the task when it's stored, deref it once the other task is done with it. you can probably solve it more intelligently than that (and should at least figure out what's going on before settling on this..!)... but it might also be useful as a temporary holdover to keep the build clean.

not sure if this will be of any use, but a comparison:

the taskgroup stuff also keeps *rust_tasks around, and has other tasks access them without reference counting. the reason this is safe is that the exclusives that contain pointers to a task all act as "barriers" to the task exiting - the exit path involves revisiting all those exclusives to remove one's own pointer from them. so the mere presence of a task pointer in a group guarantees its aliveness.

the path for killing all tasks in a group (i.e., the 'other accessing task') looks like this:

do group_arc.with |tg| {
    for each_task(tg) |task| {
        rustrt::rust_task_kill_other(task);
    }
}

early on, i imagined swapping the tg out of the arc and doing the iteration after the group_arc.with block, to avoid holding the taskgroup lock for O(n) time, but you can imagine that would be racy.

I also thought about doing that plus refcounting the pointers, but decided that was a little more complicated. it occurs to me now there's a way to do it without memory penalties - maybe for the future. something like:

let mut group = none;
do group_arc.with |tg| { *tg <-> group; }
for each_task(group) |task| {
    rustrt::rust_task_kill_other(task);
    rustrt::rust_task_deref(task);
}

[eholk]

After talking with @bblum a bit, it seems like reference counting in this case is the right way to solve the problem. Basically, you need to make sure the task stays live as long as a pointer to it is in one of the packets.