Vadim Zeitlin
2016-05-25 22:17:12 UTC
Hello,
Currently there is an enormous number of memory leaks in SWIG code as
plenty of DOH objects are never freed. It could be argued that it doesn't
really matter for a program such as SWIG which doesn't run for a long time
and so it doesn't matter if it leaks memory anyhow as all of it will be
reclaimed by the OS on process termination. However the problem is that
there are still _some_ calls to Delete() in SWIG code. And I never know if
I should care about memory being leaked and try to at least call Delete()
correctly in the new code or not.
To make this more concrete, I'm trying merging the latest master into the
doxygen branch. There are plenty of non-trivial conflicts as handling of
docstring indentation in master was completely modified in the meanwhile
(commit fa282b3540c87927fc3562bd0f0863accf6a853f and others), so I have to
change some code manually. My existing code in the branch tries to avoid
memory leaks but the new code doesn't, at all, e.g. this:
Printv(doc, triple_double, "\n",
indent_docstring(autodoc, indent), "\n",
indent_docstring(str, indent), indent, triple_double, NIL);
blithely leaks the strings returned by indent_docstring(). Now, again, I'm
not saying it's necessarily a problem (even though I do admit that it's
sometimes difficult to suppress my instinctive attempts to fix them), but
I'd like to have a confirmation that we don't care about them and then stop
using Delete() at all because it's just inconsistent and confusing to use
it in some places but not in similar situations elsewhere.
Of course, there is also the possibility of switching all this code to
more idiomatic C++ and return {auto,unique}_ptr<> instead of raw pointers
everywhere, but I don't personally have time/energy to do it and it seems
unlikely to happen.
But I do feel like we should settle on either trying to avoid leaks at
least in the new code or stop using Delete() at all and just make it a NOP.
It could probably make SWIG run faster, too...
What do you think?
VZ
Currently there is an enormous number of memory leaks in SWIG code as
plenty of DOH objects are never freed. It could be argued that it doesn't
really matter for a program such as SWIG which doesn't run for a long time
and so it doesn't matter if it leaks memory anyhow as all of it will be
reclaimed by the OS on process termination. However the problem is that
there are still _some_ calls to Delete() in SWIG code. And I never know if
I should care about memory being leaked and try to at least call Delete()
correctly in the new code or not.
To make this more concrete, I'm trying merging the latest master into the
doxygen branch. There are plenty of non-trivial conflicts as handling of
docstring indentation in master was completely modified in the meanwhile
(commit fa282b3540c87927fc3562bd0f0863accf6a853f and others), so I have to
change some code manually. My existing code in the branch tries to avoid
memory leaks but the new code doesn't, at all, e.g. this:
Printv(doc, triple_double, "\n",
indent_docstring(autodoc, indent), "\n",
indent_docstring(str, indent), indent, triple_double, NIL);
blithely leaks the strings returned by indent_docstring(). Now, again, I'm
not saying it's necessarily a problem (even though I do admit that it's
sometimes difficult to suppress my instinctive attempts to fix them), but
I'd like to have a confirmation that we don't care about them and then stop
using Delete() at all because it's just inconsistent and confusing to use
it in some places but not in similar situations elsewhere.
Of course, there is also the possibility of switching all this code to
more idiomatic C++ and return {auto,unique}_ptr<> instead of raw pointers
everywhere, but I don't personally have time/energy to do it and it seems
unlikely to happen.
But I do feel like we should settle on either trying to avoid leaks at
least in the new code or stop using Delete() at all and just make it a NOP.
It could probably make SWIG run faster, too...
What do you think?
VZ