classification
Title: platform.win32_ver() leaks in 2.7.12
Type: resource usage Stage: resolved
Components: Windows Versions: Python 3.7, Python 3.6, Python 3.5, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: steve.dower Nosy List: Okko.Willeboordse, eryksun, haypo, paul.moore, python-dev, steve.dower, tim.golden, zach.ware
Priority: normal Keywords:

Created on 2016-09-01 15:34 by Okko.Willeboordse, last changed 2016-09-21 16:08 by steve.dower. This issue is now closed.

Files
File name Uploaded Description Edit
leaked_objects.txt Okko.Willeboordse, 2016-09-01 15:34 List of leaked objects
Messages (13)
msg274144 - (view) Author: Okko Willeboordse (Okko.Willeboordse) Date: 2016-09-01 15:34
Running;
Python 2.7.12 (v2.7.12:d33e0cf91556, Jun 27 2016, 15:19:22) [MSC v.1500 32 bit (Intel)] on win32

platform.win32_ver() returns;
('10', '10.0.10586', '', u'Multiprocessor Free')
msg274145 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2016-09-01 15:35
Hum, can you please elaborate the issue? What is leaked_objects.txt?
msg274146 - (view) Author: Okko Willeboordse (Okko.Willeboordse) Date: 2016-09-01 15:38
I did a gc.get_objects() before and after platform.win32_ver() and printed
objects that were not present before calling platform.win32_ver().

If I run platform.win32_ver() in a loop I see the memory increasing.

On 1 September 2016 at 17:35, STINNER Victor <report@bugs.python.org> wrote:

>
> STINNER Victor added the comment:
>
> Hum, can you please elaborate the issue? What is leaked_objects.txt?
>
> ----------
> nosy: +haypo
>
> _______________________________________
> Python tracker <report@bugs.python.org>
> <http://bugs.python.org/issue27932>
> _______________________________________
>
msg275520 - (view) Author: Roundup Robot (python-dev) Date: 2016-09-10 01:05
New changeset 384c178cf823 by Steve Dower in branch '3.5':
Issue #27932: Fixes memory leak in platform.win32_ver()
https://hg.python.org/cpython/rev/384c178cf823

New changeset 31b7eaff5588 by Steve Dower in branch 'default':
Issue #27932: Fixes memory leak in platform.win32_ver()
https://hg.python.org/cpython/rev/31b7eaff5588
msg275521 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2016-09-10 01:06
I fixed in 3.5 and 3.6, but I'm not set up for 2.7 so someone will have to backport it.
msg275672 - (view) Author: Roundup Robot (python-dev) Date: 2016-09-10 18:54
New changeset dbfcb3b9c3c1 by Steve Dower in branch '3.5':
Issue #27932: Backs out change
https://hg.python.org/cpython/rev/dbfcb3b9c3c1
msg275673 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2016-09-10 18:54
Backed out because of issue28059. I'll rewrite this to properly use native code.
msg275715 - (view) Author: Eryk Sun (eryksun) * Date: 2016-09-10 23:44
The leak is due the pointer-type cache that ctypes.POINTER uses. The type the pointer refers to is used as the key. In this case, VS_FIXEDFILEINFO is created each time win32_ver is called, so the pointer-type cache grows without bound. Example leak:

    >>> import psutil, platform
    >>> proc = psutil.Process()
    >>> proc.memory_info().rss
    14704640
    >>> for i in range(10000):
    ...     v = platform.win32_ver()
    ...
    >>> proc.memory_info().rss
    92704768
    >>> for i in range(10000):
    ...     v = platform.win32_ver()
    ...
    >>> proc.memory_info().rss
    168861696

Clearing the cache followed by a collect() reclaims the leaked memory for the most part:

    >>> import gc, ctypes
    >>> gc.collect()
    333
    >>> proc.memory_info().rss
    168849408
    >>> ctypes._pointer_type_cache.clear()
    >>> gc.collect()
    740000
    >>> proc.memory_info().rss
    20303872

It's a moot point, since Steve plans to re-implement this check in C, but the minimal change to fix this leak is to bypass the pointer-type cache by manually subclassing ctypes._Pointer:

    class PVS_FIXEDFILEINFO(_Pointer):
        _type_ = VS_FIXEDFILEINFO

    pvi = PVS_FIXEDFILEINFO()

There's no more leak after this change:

    >>> import psutil, platform
    >>> proc = psutil.Process()
    >>> proc.memory_info().rss
    15450112
    >>> for i in range(10000):
    ...     v = platform.win32_ver()
    ...
    >>> proc.memory_info().rss
    16592896
    >>> for i in range(10000):
    ...     v = platform.win32_ver()
    ...
    >>> proc.memory_info().rss
    16601088
msg275744 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2016-09-11 08:15
> pointer-type cache grows without bound

Maybe we should also fix ctypes to use an LRU cache like
@functools.lru_cache? A cache without limit doesn't seem like a good
idea.
msg275753 - (view) Author: Eryk Sun (eryksun) * Date: 2016-09-11 09:19
Limiting the pointer-type cache could be a problem. It's expected that POINTER() returns a reference to an existing pointer type. ctypes uses Python types to ensure compatible C data types. For example:

    LP_c_int = ctypes.POINTER(ctypes.c_int)

    class LP_c_int_uncached(ctypes._Pointer):
        _type_ = ctypes.c_int

    >>> arr = (LP_c_int * 2)()
    >>> ctypes.POINTER(ctypes.c_int) is LP_c_int
    True
    >>> arr[0] = ctypes.POINTER(ctypes.c_int)()
    >>> arr[1] = LP_c_int_uncached()
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
    TypeError: incompatible types, LP_c_int_uncached instance
    instead of LP_c_int instance

The docs could warn users that the cache used by ctypes.POINTER(), and implicitly by ctypes.pointer(), will leak memory if it's called with a type that's created at function scope instead of at module or class-body scope.
msg276859 - (view) Author: Roundup Robot (python-dev) Date: 2016-09-17 23:43
New changeset 9b0d661a16de by Steve Dower in branch '2.7':
Issue #27932: Prevent memory leak in win32_ver().
https://hg.python.org/cpython/rev/9b0d661a16de
msg276861 - (view) Author: Roundup Robot (python-dev) Date: 2016-09-18 00:30
New changeset db4b254f5df4 by Steve Dower in branch '3.5':
Issue #27932: Prevent memory leak in win32_ver().
https://hg.python.org/cpython/rev/db4b254f5df4

New changeset 14fca2c8eb36 by Steve Dower in branch '3.6':
Issue #27932: Prevent memory leak in win32_ver().
https://hg.python.org/cpython/rev/14fca2c8eb36

New changeset 19ceb5034fe5 by Steve Dower in branch 'default':
Issue #27932: Prevent memory leak in win32_ver().
https://hg.python.org/cpython/rev/19ceb5034fe5
msg276862 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2016-09-18 00:32
Phew. Doing that in a reasonable way for all the active versions was a pain, but I think I managed it.

In short:
* 2.7 gets the fix proposed by Eryk Sun
* 3.5 gets a private sys.getwindowsversion()._platform_version member
* 3.6+ gets a public/documented sys.getwindowsversion().platform_version member

AFAICT the implementation is solid, but please shout if you spot anything weird/wrong.
History
Date User Action Args
2016-09-21 16:08:56steve.dowersetstatus: open -> closed
resolution: fixed
stage: commit review -> resolved
2016-09-18 00:32:51steve.dowersetstage: commit review
messages: + msg276862
versions: + Python 3.7
2016-09-18 00:30:26python-devsetmessages: + msg276861
2016-09-17 23:43:53python-devsetmessages: + msg276859
2016-09-11 09:19:11eryksunsetmessages: + msg275753
2016-09-11 08:15:52hayposetmessages: + msg275744
2016-09-10 23:44:30eryksunsetnosy: + eryksun
messages: + msg275715
2016-09-10 18:54:59steve.dowersetassignee: steve.dower
messages: + msg275673
2016-09-10 18:54:03python-devsetmessages: + msg275672
2016-09-10 01:06:01steve.dowersetmessages: + msg275521
versions: + Python 3.5, Python 3.6
2016-09-10 01:05:25python-devsetnosy: + python-dev
messages: + msg275520
2016-09-01 15:38:35Okko.Willeboordsesetmessages: + msg274146
2016-09-01 15:35:39hayposetnosy: + haypo
messages: + msg274145
2016-09-01 15:34:31Okko.Willeboordsecreate