Auto-closing the stream in __destruct() leads to invalid resource, if Stream used inside a function

[k00ni]

I'd experienced that if you using the Stream class inside a function the __destruct function will close the stream, after the function is finished, so the according file resource is not valid anymore.

Here is a small exploit:

function streamerTest($outputStream)
{
    $stream = new \Streamer\Stream($outputStream);
    $stream->write('foo' . PHP_EOL);
}

$outputStream = tmpfile();

var_dump($outputStream);
var_dump(is_resource($outputStream));

streamerTest($outputStream);

echo "\n
after function call
\n";

var_dump($outputStream);
var_dump(is_resource($outputStream));

The output is:

resource(93) of type (stream)
bool(true)

after function call

resource(93) of type (Unknown)
bool(false)

I guess that behavior is intentional and in general maybe not wrong at all. But in case, i want to reuse the resource i given to the Stream, that behavior messing with my code.

My preference would that you make it optional and you can deactivate that behavior in the class constructor via an option or something. I can help you with that, if you want.

[r4j4h]

Would a reference help in this case?
Or returning the stream object?

[k00ni]

Hi @r4j4h,

the problem appeared in a case where i wanted to use Streamer together with another tool (try to remember the name). That tool needs an open stream, but Streamer does close the stream after the object gets destructed.

My suggestion would be to make that behaviour configurable. Something like an additional parameter for the constructor for instance.

What you think?

[r4j4h]

I think that is a phenomenal suggestion and probably the easiest to implement for the most gain.

Looking at the source, Streamer is always handed a Stream and never creating it, so Streamer always destroying it takes away control from the user and in a way violates the Resource Acquisition is Initialization principle. However, it is logical and convenient that when Streamer ends the underlying Stream should probably also end - that just doesn't cover every use case..

That brings up the consideration of what if someone prematurely ends a stream given to Streamer. Luckily it's already handling that with its assertReadable function.

Lastly, I see some consideration around Streamer leaking a Stream that never gets properly closed if this functionality is disabled and an unexpected exception occurs. Probably unlikely, and some documentation warning users of the risk is probably enough, but it warrants mentioning.

Good call. :)