reactphp / filesystem

Evented filesystem access.
MIT License
135 stars 40 forks source link

Error when copying file to file #18

Open seregazhuk opened 6 years ago

seregazhuk commented 6 years ago

I've tried to run this example with copying one file to another. It fails with:

Notice: Undefined offset: 1 in vendor/react/filesystem/src/ChildProcess/Adapter.php on line 225

I thinks this happens because we try to close() destination file twice. The first attempt happens here, when are done with reading:

https://github.com/reactphp/filesystem/blob/47d1b1857a175c04dbe88fd02a6cbb56924fc357/src/Node/File.php#L216-L221

We call end() which implicitly closes the file.

And then in always() handler:

https://github.com/reactphp/filesystem/blob/47d1b1857a175c04dbe88fd02a6cbb56924fc357/src/Node/File.php#L227-L229

We again try to close the same file and it fails. Should this always handler be removed or there is another solution?

hotrush commented 6 years ago

Have the same bug with child process adapter, happens randomly when writing lot of files, but data written correctly.

WyriHaximus commented 6 years ago

What version are you on @hotrush? We released v0.1.1 two weeks a go which contains a lot of bug fixes.

hotrush commented 6 years ago

I'm on latest v0.1.1

WyriHaximus commented 6 years ago

Same as @seregazhuk when you use the example it fails?

hotrush commented 6 years ago

@WyriHaximus my snippet is some different, i am using it with periodic timer

<?php

require __DIR__ . '/vendor/autoload.php';
$loop = \React\EventLoop\Factory::create();

$contents = '123';

$adapter = new \React\Filesystem\ChildProcess\Adapter($loop);
$filesystem = \React\Filesystem\Filesystem::createFromAdapter($adapter);
echo 'Using ', get_class($filesystem->getAdapter()), PHP_EOL;

$j = 0;

$loop->addPeriodicTimer(1, function () use ($filesystem, $contents, &$j) {
    for ($i = $j; $i <= $j+9; $i++) {
        $filename = __DIR__ . '/data/test/'.$i.'.txt';
        $filesystem->file($filename)->putContents($contents)->then(function () use ($filename) {
            echo $filename.' written'.PHP_EOL;
        }, function (Exception $e) {
            echo $e->getMessage(), PHP_EOL;
            echo $e->getTraceAsString(), PHP_EOL;
        });
    }
    $j += 10;
});

$loop->run();
vagrant@jessie:~/test$ php test.php 
Using React\Filesystem\ChildProcess\Adapter
/home/vagrant/test/data/test/0.txt written
/home/vagrant/test/data/test/1.txt written
/home/vagrant/test/data/test/2.txt written
/home/vagrant/test/data/test/4.txt written
/home/vagrant/test/data/test/3.txt written
/home/vagrant/test/data/test/5.txt written
/home/vagrant/test/data/test/6.txt written
/home/vagrant/test/data/test/8.txt written
PHP Notice:  Undefined offset: 1 in /home/vagrant/test/vendor/react/filesystem/src/ChildProcess/Adapter.php on line 219
/home/vagrant/test/data/test/9.txt written
/home/vagrant/test/data/test/10.txt written
/home/vagrant/test/data/test/13.txt written
/home/vagrant/test/data/test/11.txt written
/home/vagrant/test/data/test/12.txt written
/home/vagrant/test/data/test/14.txt written
/home/vagrant/test/data/test/15.txt written
/home/vagrant/test/data/test/16.txt written
/home/vagrant/test/data/test/18.txt written
/home/vagrant/test/data/test/17.txt written
/home/vagrant/test/data/test/19.txt written
/home/vagrant/test/data/test/20.txt written
/home/vagrant/test/data/test/23.txt written
/home/vagrant/test/data/test/21.txt written
/home/vagrant/test/data/test/22.txt written
/home/vagrant/test/data/test/24.txt written
/home/vagrant/test/data/test/26.txt written
/home/vagrant/test/data/test/25.txt written
/home/vagrant/test/data/test/28.txt written
/home/vagrant/test/data/test/27.txt written
/home/vagrant/test/data/test/29.txt written
/home/vagrant/test/data/test/31.txt written
/home/vagrant/test/data/test/30.txt written
/home/vagrant/test/data/test/33.txt written
/home/vagrant/test/data/test/32.txt written
/home/vagrant/test/data/test/34.txt written
/home/vagrant/test/data/test/35.txt written
/home/vagrant/test/data/test/37.txt written
/home/vagrant/test/data/test/36.txt written
/home/vagrant/test/data/test/38.txt written
/home/vagrant/test/data/test/39.txt written
/home/vagrant/test/data/test/40.txt written
/home/vagrant/test/data/test/41.txt written
/home/vagrant/test/data/test/43.txt written
/home/vagrant/test/data/test/42.txt written
/home/vagrant/test/data/test/44.txt written
/home/vagrant/test/data/test/45.txt written
/home/vagrant/test/data/test/47.txt written
/home/vagrant/test/data/test/46.txt written
/home/vagrant/test/data/test/49.txt written
/home/vagrant/test/data/test/48.txt written
/home/vagrant/test/data/test/50.txt written
/home/vagrant/test/data/test/51.txt written
/home/vagrant/test/data/test/52.txt written
/home/vagrant/test/data/test/53.txt written
/home/vagrant/test/data/test/54.txt written
/home/vagrant/test/data/test/55.txt written
PHP Notice:  Undefined offset: 1 in /home/vagrant/test/vendor/react/filesystem/src/ChildProcess/Adapter.php on line 219
/home/vagrant/test/data/test/56.txt written
/home/vagrant/test/data/test/58.txt written
/home/vagrant/test/data/test/59.txt written
/home/vagrant/test/data/test/60.txt written
/home/vagrant/test/data/test/61.txt written
/home/vagrant/test/data/test/62.txt written
PHP Notice:  Undefined offset: 2 in /home/vagrant/test/vendor/react/filesystem/src/ChildProcess/Adapter.php on line 219
/home/vagrant/test/data/test/63.txt written
/home/vagrant/test/data/test/65.txt written
/home/vagrant/test/data/test/66.txt written
/home/vagrant/test/data/test/68.txt written
/home/vagrant/test/data/test/67.txt written
/home/vagrant/test/data/test/69.txt written
/home/vagrant/test/data/test/71.txt written
/home/vagrant/test/data/test/70.txt written
PHP Notice:  Undefined offset: 1 in /home/vagrant/test/vendor/react/filesystem/src/ChildProcess/Adapter.php on line 219
/home/vagrant/test/data/test/73.txt written
/home/vagrant/test/data/test/75.txt written
/home/vagrant/test/data/test/74.txt written
/home/vagrant/test/data/test/77.txt written
/home/vagrant/test/data/test/78.txt written
/home/vagrant/test/data/test/79.txt written
/home/vagrant/test/data/test/76.txt written
^C
seregazhuk commented 6 years ago

I cannot reproduce my issue with a new version v0.1.1 👌

WyriHaximus commented 6 years ago

@hotrush thanks, will go over it soon and see where things have to be improved. @seregazhuk great to hear!

mmoreram commented 3 years ago

I still have this issue. Do you have any updates?