Volatile isn't thread safe

[joeyhub]

I have these two methods both of which run syncronously:

public function push($value):void {
    $this->data[] = $value;
}

public function drain():array {
    $data = $this->data;
    $this->data = new Volatile();

    return (array)$data;
}

When my thread calls drain, for a little bit the volatile object in data are fine but then they just disappear in the middle of processing becoming empty volatiles. Usually while another thread is pushing to the new Volatile.

[joeyhub]

Double bufferring appears to avoid the issue:

private $data = [[], []];
private $i = 0;

public function push($value):void {
    $this->data[$this->i][] = $value;
}

public function drain():Volatile {
    $data = $this->data[$this->i];
    $this->data[$this->i] = new Volatile();
    $this->i = +!$this->i;

    return $data;
}

public function shouldWait():bool {
    return count($this->data[$this->i]) === 0;
}

In some cases it's even weird, an array gets misplaced in the volatile. Someone is definitely screwing around with reused pointers there.

[sirsnyder]

Could you provide a runnable example? Thx!

[joeyhub]

Here: https://github.com/joeyhub/aphpsync/blob/master/prototypes/test.php

[joeyhub]

It's worth pointing out that I have a ref issue there. Eventually a command is sent to a thread which holds a reference up to a thread to notify when done. Nothing references the child threads, instead everything references the man in the middle.

I've been meaning to fix that later for graceful completion. I didn't really consider it related as I would expect it to hang on wait and it's been doing something weird hanging somewhere else. Though at first I didn't consider them related and would really expect a segfault, hang on wait or error, there's a good chance they are related as volatile's weirdness seems to set in a around about where the refs are gone and also if the handling of lost ref (hanging in weird places) is broken somehow then it wouldn't be surprising if it has other problems.

Still a bit of a headscratcher that double bufferring improves it (should remove any possibility for concurrent access in this particular use case) though I think it just reduces it.

[joeyhub]

Adding graceful (though leaky) thread shutdown does appear to improve a lot of secondary issues I've been encountering with volatile even after the double bufferring.

However I'm not sure how reliable that graceful shutdown is. If the main thread dies it looks as though things will probably just hang. Similarly I practically need a thread monitor to potentially handle dead threads (try catch isn't reliable enough in my book either). I guess if thread death notified perhaps that could be enough though kind of ugly.

Worse still is tuning up the concurrency (workload per thread) and I got it to segfault. Whatever it is it's about one in 16000.

[ghost]

Honestly I'd just do while($this->data->shift()); for draining.

[joeyhub]

I've considered that although it's not really documented. Even then things shouldn't be hanging and segfaulting as they are. The current release definitely isn't stable or trustable.

Also relying on type juggling there isn't necessarily safe unless you always wrap the data in something that juggles to true. Similarly it doesn't seem to perform well that I would have to iterate if access to that needs to be synchronised.

Looking at the source those operations are only valid for Volatile anyway. Chunk actually deletes apparently so should probably be faster. Shifting each one isn't going to be all that efficient. Chunk should at least for example only need to lock once.