rockettheme / toolbox

RocketTheme\Toolbox package contains a set of reusable PHP interfaces, classes and traits.
MIT License
21 stars 16 forks source link

File locking problem on Windows XAMPP #19

Open Perlkonig opened 6 years ago

Perlkonig commented 6 years ago

In my Grav plugin Count Views, I'm using the RocketTheme\Toolbox\File\File interface. The plugin works fine in *nix environments, but it doesn't work in Windows. (I've only tested under XAMPP. But I'm told it works under the Windows Subsystem for Linux.)

The problem is these lines here:

        // Open data file
        $datafh = File::instance($path);
        $datafh->lock();
        $data = Yaml::parse($datafh->content());
        if ($data === null) {
            $data = array();
        }

No matter what, $datafh->content() is returning null on Windows, even though it loaded the file fine on lines 63-66. The only difference is the file locking.

I added an if/then, and indeed the file is getting locked. And I can confirm that if I remove the lock() line, everything works as expected.

Any help appreciated. Thanks!

Perlkonig commented 6 years ago

Just following up. Anyone around?

mahagr commented 6 years ago

Not sure why I have missed your first post, maybe I accidentally marked it as read.

Try if using CompiledYamlFile class works better. It handles yaml decoding/encoding for you, so you can just pass the data to it. It also always returns an array, even if the file does not exist. It is also much faster and doesn't seem to be causing any issues.

You should also test if the locking succeeded as it can fail and you need to deal with it. Umm, does the locking always fail?

Perlkonig commented 6 years ago

The lock reports as successful but the data still reads as null.

I will try the different class this weekend and report back. Thanks.

Perlkonig commented 6 years ago

You're referring to the Grav class CompiledYamlFile I presume?

While that should work fine for loading the file to get the views, it's not sufficient for recording the view in the first place. Perhaps I'm not conceptualizing things correctly, though.

When recording the view, I need lock the file for longer than just reading the contents. I need to keep the file locked until I record the view. Otherwise, in a flurry of activity, counts may get clobbered. So I think I still need the explicit lock() and free() calls.

mahagr commented 5 years ago

Sorry, I've been super busy.

Yes, I'm referring Grav class CompiledYamlFile as it seems to be fine. The only difference to the YamlFile class is that it stored PHP file in addition to YAML file, making a lot faster. It also seems to overcome the issue with YAML file locking in windows as it uses opcache invalidation.

And yes, it should work with the locking.