[issue29034] Fix memory leak in path_converter

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

There are many ways to write a solution of the original issue. You can just add 
a number of "Py_DECREF(bytes)" (path_converter.patch), or add new variable 
to_cleanup2 (path_converter-v2.patch), or reuse to_cleanup 
(path_converter-to_cleanup.patch), or clean ups bytes 
(path_converter-bytes.patch). All these ways look equivalent.

The issue mentioned in msg283754 is more severe. It can be solved by making 
path->object referring an original argument (but some code checks the type of 
path->object), or making path->object owning a reference.

It seems to me that path->narrow can be not initialized for str path on Windows.

--
nosy: +brett.cannon
Added file: http://bugs.python.org/file45985/path_converter-to_cleanup.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29034] Fix memory leak in path_converter

[Serhiy Storchaka]

Changes by Serhiy Storchaka :


Added file: http://bugs.python.org/file45986/path_converter-bytes.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29034] Fix memory leak in path_converter

[STINNER Victor]

STINNER Victor added the comment:

> The logic is complex ...

Yeah, that's why I first proposed to add a dummy to_cleanup2 object.

Another option is to use your first patch.

I don't care much about the exact implementation :-) Maybe Serhiy has
a better understanding of the code and can help to decide on this
issue ;-)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29034] Fix memory leak in path_converter

[Xiang Zhang]

Xiang Zhang added the comment:

Hmm, while considering Victor's comment, I find some new:

path->object refers to the original object o, it owns a borrowed reference. But 
when received a PathLike object, o is assigned the return value of __fspath__ 
and decrefed at end. So path->object could refer to a already freed object.

And the PyUnicode_AsWideCharString can't be simply replaced with 
PyUnicode_AsUnicodeAndSize since wo is decrefed and object->wide could then 
refer to freed memory.

The logic is complex and I get headache reading the code. Did I miss something?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29034] Fix memory leak in path_converter

[STINNER Victor]

STINNER Victor added the comment:

> You could just reuse to_cleanup instead of to_cleanup2.

Ah, it would be better to avoid a new variable, but this code is too complex 
for my head. I don't understand what is to_cleanup in this code path :-)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29034] Fix memory leak in path_converter

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

You could just reuse to_cleanup instead of to_cleanup2.

--
nosy: +serhiy.storchaka

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29034] Fix memory leak in path_converter

[Xiang Zhang]

Xiang Zhang added the comment:

v2 applies the comments. And another separate change is to change 
PyUnicode_AsWideCharString to PyUnicode_AsUnicodeAndSize.

--
title: refleak in path_converter on error case -> Fix memory leak in 
path_converter
Added file: http://bugs.python.org/file45983/path_converter-v2.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com