Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PHP Warning: fclose(): supplied resource is not a valid stream resource #311

Closed
IT-Experte opened this issue Dec 14, 2016 · 7 comments
Closed
Labels

Comments

@IT-Experte
Copy link

IT-Experte commented Dec 14, 2016

PHP 7.0.8
mongo-php-library 1.1.0 (final)

PHP Warning: fclose(): supplied resource is not a valid stream resource in .../src/GridFS/ReadableStream.php on line 78

This is my php code. It reads images from a MongoDB collection and display each file.

<?php
// include Composer (mongodb) extension
require '/src/composer/vendor/autoload.php';

class GridFS {

        private $client;

        public function __construct() {
                $this->client = new MongoDB\Client("mongodb://localhost:27017");
        }

        public function Image($id) {

                $db = $this->client->selectDatabase("myfiles");
                $bucket = $db->selectGridFSBucket();

                $stream = $bucket->openDownloadStream($id);
                header("Content-type: image/jpg;");
                echo stream_get_contents($stream);
                exit;
        }

        public function FindFiles($limit) {

                $collection = $this->client->selectCollection("myfiles", "fs.files");
                $filter = [];
                $options = ['limit' => $limit];
                return $collection->find($filter, $options);
        }
}

$id = isset($_GET['id']) ? urldecode($_GET['id']) : null;


$gfs = new GridFS();

if(isset($id)) {
        $gfs->Image($id);
}

?>

<html>
<head><title>ImageFiles</title></head>
<body>

<?php

$files = $gfs->FindFiles(10);

foreach ($files as $file) {
        $id = $file["_id"];
        echo "<h2>ID: $id</h2>\r\n";
        echo "<img src='?id=$id' border='0' alt='' /><br /> \r\n";
}
?>

</body>
</html>

My solution is, to hide the warning...

public function close()
{
    if (is_resource($this->buffer)) {
        @fclose($this->buffer);
    }
}
IT-Experte added a commit to IT-Experte/mongo-php-library that referenced this issue Dec 14, 2016
@jmikola
Copy link
Member

jmikola commented Dec 14, 2016

Can you share a PHP script that reproduces this warning?

Based on the code, it's not clear to me how this warning would be triggered unless ReadableStream::close() was invoked multiple times, which itself sounds like a bug in our StreamWrapper implementation.

@IT-Experte
Copy link
Author

I have inserted my example php code. You need some image files into mongo database "myfiles". I think, Close(); was invoked multiple times (for each image).

@jmikola
Copy link
Member

jmikola commented Dec 15, 2016

I was able to test this a bit and it appears to be specific to PHP 7.x. Our StreamWrapper's close() method is still only called once, but the in-memory stream we use for buffering appears to be closed before the GridFS stream that encapsulates it. When the exit; statement is moved out of the Image() method, the script completes without any warning:

if(isset($id)) {
    $gfs->Image($id);
    exit; // relocated from GridFS::Image()
}

This makes sense, as our stream goes out of scope when Image() returns and is closed naturally.

I'm currently in the midst of refactoring the StreamWrappers to not use streams for buffer (see: #304), so this will be resolved. In the meantime, feel free to use the above work-around.

@jmikola jmikola added the bug label Dec 15, 2016
@IT-Experte
Copy link
Author

Many thanks for your help. I've looked your refactoring and your benchmarks. It is surprising that a string buffer is faster than writing it directly into RAM.

@jmikola
Copy link
Member

jmikola commented Dec 16, 2016

It is surprising that a string buffer is faster than writing it directly into RAM.

Strings are in RAM as well. I was a bit surprised to find that the php://memory stream was nearly 4x slower, but I suppose that may be overhead of PHP's APIs (e.g. fwrite()). There is definitely a use case for php://memory, as in our tests when we want to quickly feed strings into GridFS without creating a file on disk; however, there wasn't a compelling reason to keep it in our StreamWrappers.

@jmikola
Copy link
Member

jmikola commented Jan 6, 2017

This should be fixed by #304 and will be resolved in the next 1.1.x release.

@jmikola jmikola closed this as completed Jan 6, 2017
@jmikola
Copy link
Member

jmikola commented Jan 12, 2017

Correction: #304 pertained only to WritableStream refactoring. This was actually resolved by ReadableStream refactoring in #323 (PHPLIB-247).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants