Changes between Initial Version and Version 1 of Ticket #17258, comment 5


Ignore:
Timestamp:
Dec 12, 2011, 12:00:06 PM (13 years ago)
Author:
Anssi Kääriäinen

Legend:

Unmodified
Added
Removed
Modified

  • Ticket #17258, comment 5

    initial v1  
    22
    33About the garbage collection risk, there are a couple of things to note:
    4   1. The garbage collection closes the underlying connection, so you can also have a connection leak if the GC fails to collect the connection object.
    5   2. Circular references + `__del__` make the cycle non-collectable. See: [http://docs.python.org/library/gc.html#gc.garbage].
     4  1. The garbage collection closes the underlying connection, so you would a connection leak if the GC fails to collect the connection object.
     5  2. Circular references + `__del__` make objects in cyclic references non-collectable. See: [http://docs.python.org/library/gc.html#gc.garbage].
    66  3. However, if the cycle is caused by a threading.local object, 2. isn't in effect. (Yes, this is weird).
    7   4. On cpython the timing of the GC is altered. Non-cyclic (or cycles containing threading.local objects) are collected immediately, but objects with referential cycles only when the GC happens to run next time. On `PyPy` for example, the objects are collected only when the garbage collector happens to run, so no change for `PyPy` users. In general, if your code relies on the exact timing of GC, you are doing it wrong.
     7  4. On standard Python the timing of the garbage collection is altered. Non-cyclic (or cycles containing threading.local objects) are collected immediately, but objects with referential cycles only when the GC happens to run next time. On `PyPy`, the objects are collected only when the garbage collector happens to run, so no change for `PyPy` users. In general, if your code relies on the exact timing of GC, you are doing it wrong.
    88
    9 It so happens that the `BaseDatabaseWrapper` is currently threading.local, and has referential cycles (wrapper -> introspection -> wrapper etc). So, by removing the threading.local, we remove rule 3, and rule 2 is in effect. Luckily there is no `__del__` defined for the wrapper, so we should be relatively safe, except for the timing. External backends which define `__del__` would have a really hard bug to debug.
     9It so happens that the `BaseDatabaseWrapper` is currently threading.local, and has referential cycles (wrapper -> introspection -> wrapper etc). So, by removing the threading.local, we remove rule 3, and rule 2 is in effect. Luckily there is no `__del__` defined for the wrapper, so we should be relatively safe, except for the altered timing. External backends which define `__del__` would have a really hard bug to debug.
    1010
    11 This should not affect web-serving at all, as after each request connections are closed explicitly. But testing & scripts and things like that could have a problem.
     11This should not affect web-serving at all, as after each request connections are closed explicitly. But testing & scripts and things like that could potentially have a problem.
    1212
    13 The long-term solution would be to break the cycles in `BaseDatabaseWrapper`. However that is going to be an invasive patch, which will break most external backends.
     13The long-term solution would be to break the cycles in `BaseDatabaseWrapper`. However that is going to be an invasive patch, and would break most external backends.
    1414
    15 All in all, I think the change should be safe from GC point of view. The patch looks good (on a quick glance), except potentially for the duplicate threading.local.
     15All in all, I think the patch should be safe from GC point of view. The patch looks good (on a quick glance), except potentially for the duplicate threading.local.
Back to Top