Description
task::unkillable is used a bunch in task, some in sync, and I suspect it really should be used in other places where we have unsafe code, like pipes.
Currently it hammers on lifecycle_lock every time:
void rust_task::inhibit_kill() {
scoped_lock with(lifecycle_lock);
disallow_kill++;
}
void rust_task::allow_kill() {
scoped_lock with(lifecycle_lock);
disallow_kill--;
}
The reason is to protect it from racing a killer (this code is approximate):
void rust_task::kill() {
scoped_lock with(lifecycle_lock);
killed = true;
if (blocked && disallow_kill == 0) {
// Point A
punt_awake();
}
}
If we just removed the lock in inhibit_kill, the race could be that at point A, the unkillable task increments its flag and promptly goes to sleep, only to get punted awake from what it thought was an unkillable sleep.
If we removed the lock in allow_kill, the race would be that the killer sees the task is not killable, and at point A it then becomes killable and goes to sleep, but the killer doesn't punt it awake, and it blocks forever.
I believe the following version should be safe (though not helgrind-clean), and reduce traffic on lifecycle_lock:
void rust_task::inhibit_kill() {
disallow_kill++;
if (killed) {
scoped_lock with(lifecycle_lock);
}
}
void rust_task::allow_kill() {
disallow_kill--;
if (killed) {
scoped_lock with(lifecycle_lock);
}
}
Try this and see what it does to task and sync performance. This is pretty fragile code (i.e. might start racing if something changes elsewhere), so only actually use it if it's a clear win.