Issue 27932: platform.win32_ver() leaks in 2.7.12

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')

Hum, can you please elaborate the issue? What is leaked_objects.txt?

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>
> _______________________________________
>

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

I fixed in 3.5 and 3.6, but I'm not set up for 2.7 so someone will have to backport it.

New changeset dbfcb3b9c3c1 by Steve Dower in branch '3.5':
Issue #27932: Backs out change
https://hg.python.org/cpython/rev/dbfcb3b9c3c1

Backed out because of issue28059. I'll rewrite this to properly use native code.

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

> 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.

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.

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

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

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.