php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #78930 PDO Sqlite not close connection after destruction (windows only)
Submitted: 2019-12-08 18:25 UTC Modified: 2019-12-09 08:40 UTC
From: julien dot boudry at gmail dot com Assigned:
Status: Not a bug Package: PDO SQLite
PHP Version: 7.4.0 OS: Windows only
Private report: No CVE-ID: None
 [2019-12-08 18:25 UTC] julien dot boudry at gmail dot com
Description:
------------
Impossible to unlink an SQLite file after destroying his PDO object. Not reproducible in any context, but happen with circular references.

On Windows only, PHP 7.3 and PHP 7.4.0 (not tested on lower versions).

Test script:
---------------
<?php

class Test {

    public static $path;

    protected $base;

    protected $prepare = [];

    protected $myContainer;

    public function __construct (Container $myContainer)
    {
        $this->myContainer = $myContainer;

        self::$path = getcwd().'/base.sqlite';

        $this->base = new PDO ('sqlite:'.self::$path,'','',[\PDO::ATTR_PERSISTENT => false]);

        $this->base->query('CREATE TABLE IF NOT EXISTS test (col_a INTEGER PRIMARY KEY NOT NULL)');

        $this->prepare[] = $this->base->prepare('INSERT INTO test VALUES (1)');
    }
}

class Container
{
    protected $myTest;

    public function __construct () {
        $this->myTest = new Test ($this);
    }
}

$container = new Container;

unset($container);

unlink(Test::$path);

Expected result:
----------------
Not any output or warning and File bdd.sqlite destroy. On Linux it's working as expected.

Actual result:
--------------
PHP Warning:  unlink(C:\php-script/base.sqlite): Resource temporarily unavailable in C:\php-script\pdo.php on line 40
PHP Stack trace:
PHP   1. {main}() C:\php-script\pdo.php:0
PHP   2. unlink() C:\php-script\pdo.php:40

Warning: unlink(C:\php-script/base.sqlite): Resource temporarily unavailable in C:\php-script\pdo.php on line 40

Call Stack:
    0.4049     396904   1. {main}() C:\php-script\pdo.php:0
    0.4073     398376   2. unlink() C:\php-script\pdo.php:40


Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2019-12-08 18:39 UTC] julien dot boudry at gmail dot com
If I comment the line 17:
// $this->myContainer = $myContainer;

It's working.

--

Even if I add a destructor on Test class:
    public function __destruct () {
        $this->myContainer = null;
    }

If not working too.
 [2019-12-08 19:12 UTC] requinix@php.net
-Status: Open +Status: Not a bug
 [2019-12-08 19:12 UTC] requinix@php.net
Windows does not allow deleting files while there are open handles to it. Linux does.

Clean up the circular reference with gc_collect_cycles().
https://www.php.net/manual/en/features.gc.php
 [2019-12-08 19:22 UTC] julien dot boudry at gmail dot com
Hi,

Even if I add gc_collect_cycles(); just before last line with unlink call. It's not working better. I do not seem to have any convincing solution for my use case.

I would like to make it clear that if I do not produce any circular references. Then the unlink works correctly under windows.
 [2019-12-08 19:27 UTC] requinix@php.net
> Even if I add gc_collect_cycles(); just before last line with unlink call. It's not working better.
> I do not seem to have any convincing solution for my use case.
Adding gc_collect_cycles() works for me. Are you running the *exact* code you posted but with a
  gc_collect_cycles();
added immediately before the unlink()?
 [2019-12-08 19:39 UTC] julien dot boudry at gmail dot com
You're right, exactly the same, it's working.

But. If I had a destructor (and change a visibility), it fails again.

Look only the last revision here (or the last version).
https://gist.github.com/julien-boudry/6672e4e98e48b14fc6a6871942a6ef2b/revisions
 [2019-12-08 19:55 UTC] requinix@php.net
I see now, the behavior is a bit different.

That prepared statement you created is keeping the SQLite resource in use. You have to unset $base as well as $prepare.
 [2019-12-08 20:08 UTC] julien dot boudry at gmail dot com
Okay, I see and it works in this simple example.

But if the use of gc_collect_cycles() already appears a bit like a hack.
That the addition of the presence of a destructor changes the way properties are cleaned still seems to me to be counter-intuitive (and not documented to my knowledge).

Now, in a much more complex code like the one I initially had a problem with. With many circular references and destructors everywhere. The problem is then more serious.
I had already tried the gc_collect_cycles() without success.  But I can't clean all the properties by hand, because it makes the code unnecessarily verbose, but also because the call order of the destroyers is not guaranteed "in any order during the shutdown sequence. " (from official doc.) and that point  produce others problems.
 [2019-12-08 20:32 UTC] requinix@php.net
I can't explain why the presence of a destructor, even an empty one, changes the behavior, or why it is apparently new to 7.4, so that could be a bug... but I feel we've digressed too far from the original purpose of this ticket, so I'll draw up a simpler repro script and create a new bug report. It may be a matter for documentation.

That aside, general programming practice with destructors is that, if you use them, you must clean up after yourself as necessary, and if I unset($this->myTest) in the destructor (like PHP would have done itself) then it works.


Regarding cycles,
1. If your code is too complicated to maintain then you need to refactor.
2. gc_collect_cycles() isn't a hack - it's the answer.

Cyclic references are a problem for any garbage collector, and the effort required to find those cycles is too much for PHP to tackle aggressively like it does noncyclic references; after all, the engine is designed to be used for short-lived requests where having a few KB of unreclaimed memory for a couple milliseconds is not a problem.
As a developer, when you build larger and larger applications you become more and more responsible for how it runs - you can't expect PHP to solve all your problems for you because it looks like a high-level language.
 [2019-12-08 20:38 UTC] julien dot boudry at gmail dot com
Thank you for your support.
We actually put our finger on at least one point that doesn't make sense, so I'll let you reformulate it it in a new ticket. You have more skills than I do to do it well ;-)
 [2019-12-08 21:56 UTC] requinix@php.net
-> bug #78933
 [2019-12-09 08:40 UTC] nikic@php.net
As I commented on bug #78933, I believe this bug illustrates ones again that we need a PDO->close() method. This is tracked at bug #62065.
 
PHP Copyright © 2001-2020 The PHP Group
All rights reserved.
Last updated: Sun May 31 20:01:27 2020 UTC