msg100462 - (view) |
Author: Stefan Holek (stefanholek) |
Date: 2010-03-05 10:27 |
The history state must be freed, like so:
diff --git a/rl/readline.c b/rl/readline.c
index a676892..2394dca 100644
--- a/rl/readline.c
+++ b/rl/readline.c
@@ -576,9 +576,12 @@ static PyObject *
get_current_history_length(PyObject *self, PyObject *noarg)
{
HISTORY_STATE *hist_st;
+ PyObject *length;
hist_st = history_get_history_state();
- return PyInt_FromLong(hist_st ? (long) hist_st->length : (long) 0);
+ length = PyInt_FromLong(hist_st ? (long) hist_st->length : (long) 0);
+ free(hist_st);
+ return length;
}
PyDoc_STRVAR(doc_get_current_history_length,
|
msg100492 - (view) |
Author: Alexander Belopolsky (Alexander.Belopolsky) |
Date: 2010-03-05 17:06 |
Note the following comment elsewhere in Modules/readline.c:
/* the history docs don't say so, but the address of state
changes each time history_get_history_state is called
which makes me think it's freshly malloc'd memory...
on the other hand, the address of the last line stays the
same as long as history isn't extended, so it appears to
be malloc'd but managed by the history package... */
free(state);
It is indeed not documented that state is malloced, but the implementation of history_get_history_state () is as follows:
/* Return the current HISTORY_STATE of the history. */
HISTORY_STATE *
history_get_history_state ()
{
HISTORY_STATE *state;
state = (HISTORY_STATE *)xmalloc (sizeof (HISTORY_STATE));
state->entries = the_history;
state->offset = history_offset;
state->length = history_length;
state->size = history_size;
state->flags = 0;
if (history_stifled)
state->flags |= HS_STIFLED;
return (state);
}
xmalloc () is an error checking wrapper around malloc, so free () can be used to deallocate the memory.
On the other hand it seems wasteful to request full state in a function that only needs history_length which is directly exported by the readline library. I am attaching a patch that reads history_length directly.
|
msg100495 - (view) |
Author: Alexander Belopolsky (Alexander.Belopolsky) |
Date: 2010-03-05 17:59 |
Not directly related to the issue at hand, but I've noticed that while readline.get_current_history_length() is tested in the unittests, readline.get_history_length() is not. Attached patch adds tests for reading and writing history files.
Also, I find it confusing that readline module static variable _history_length is named almost the same as readline library exported variable history_length. Maybe _history_length could be renamed to max_history_file_length.
|
msg100553 - (view) |
Author: Alexander Belopolsky (Alexander.Belopolsky) |
Date: 2010-03-07 00:00 |
It appears that the tests that I attached fail when libedit is used. This is clearly due to bugs in libedits readline emulation:
1. read_history does not update history_length
2. history_truncate_file does not preserver the history cookie ("_HiStOrY_V2_").
Does anyone know where libedit bugs should be reported?
|
msg100566 - (view) |
Author: Ronald Oussoren (ronaldoussoren) *  |
Date: 2010-03-07 07:22 |
On 7 Mar, 2010, at 1:02, Alexander Belopolsky wrote:
>
>
> Does anyone know where libedit bugs should be reported?
Apple's bugreporter. (bugreport.apple.com, you need an ADC account to report bugs).
|
msg100764 - (view) |
Author: Alexander Belopolsky (Alexander.Belopolsky) |
Date: 2010-03-09 22:06 |
I my experience, reporting bugs in open source components of OSX to bugreport.apple.com is a waste of time. Such reports are largely ignored and they are not visible to upstream developers.
I believe the upstream for libedit is NetBSD, http://cvsweb.netbsd.org/bsdweb.cgi/src/lib/libedit, but I cannot find their bug tracker.
|
msg100766 - (view) |
Author: Ronald Oussoren (ronaldoussoren) *  |
Date: 2010-03-09 22:15 |
Without filing a bug Apple won't know that something is wrong and they will definitly not fix the issue. If you file an issue and post the radar number I'll ping the Python maintainer inside Apple. There's little change that this will be fixed, but you never know.
|
msg100768 - (view) |
Author: Alexander Belopolsky (Alexander.Belopolsky) |
Date: 2010-03-09 22:49 |
I submitted two bug reports:
7734961
libedit history_truncate_file () fails to preserve magic line
Mac OS X
Other Bug
09-Mar-2010 05:48 PM
Open
7734839
libedit read_history() does not update history_length
Mac OS X
Other Bug
09-Mar-2010 05:39 PM
Open
|
msg112367 - (view) |
Author: Mark Dickinson (mark.dickinson) *  |
Date: 2010-08-01 18:57 |
See also issue 9450 (leaks in readline.replace_history_item and readline.remove_history_item).
|
msg112631 - (view) |
Author: Mark Dickinson (mark.dickinson) *  |
Date: 2010-08-03 16:57 |
I've fixed this leak in r83670 through r83672. It's still using the old, inefficient method (get the state, read the length, free the state), because without good tests I don't want to disturb things too much. In particular, it's not clear whether libedit keeps the history_length variable properly updated.
There are still the remaining issues of:
- better testing for the readline module, and
- attempting to work around libedit bugs.
Perhaps those should become separate issues, though?
|
msg134340 - (view) |
Author: Antoine Pitrou (pitrou) *  |
Date: 2011-04-24 19:37 |
For the record, I tried Alexander's tests on the buildbots and it failed on OS X Leopard:
http://www.python.org/dev/buildbot/all/builders/AMD64%20Leopard%20custom/builds/12/steps/test/logs/stdio
but succeeded on OS X Tiger:
http://www.python.org/dev/buildbot/all/builders/x86%20Tiger%20custom/builds/18/steps/test/logs/stdio
|
msg145320 - (view) |
Author: Enji Cooper (ngie) * |
Date: 2011-10-11 00:59 |
As a note for future reference, FreeBSD/NetBSD/OpenBSD doesn't use the term "bug", but instead uses the term "problem report" (the NetBSD website says "bug" though BTW).
The PR system for NetBSD can be accessed here: http://www.netbsd.org/cgi-bin/sendpr.cgi?gndb=netbsd .
|
msg228811 - (view) |
Author: Terry J. Reedy (terry.reedy) *  |
Date: 2014-10-08 19:23 |
The leak had been fixed in #9450.
"There are still the remaining issues of:
- better testing for the readline module, and
- attempting to work around libedit bugs.
Perhaps those should become separate issues, though?"
If those are current issues and anyone cares enough, yes, separate issues.
|
|
Date |
User |
Action |
Args |
2022-04-11 14:56:58 | admin | set | github: 52312 |
2014-10-08 19:23:57 | terry.reedy | set | status: open -> closed
superseder: readline.replace_history_item leaks memory.
nosy:
+ terry.reedy messages:
+ msg228811 resolution: duplicate stage: resolved |
2011-11-29 06:24:40 | ezio.melotti | set | versions:
- Python 3.1 |
2011-10-11 00:59:08 | ngie | set | nosy:
+ ngie messages:
+ msg145320
|
2011-04-24 19:37:39 | pitrou | set | nosy:
+ pitrou
messages:
+ msg134340 versions:
+ Python 3.3 |
2010-08-03 16:57:08 | mark.dickinson | set | messages:
+ msg112631 versions:
+ Python 3.1, Python 2.7, Python 3.2 |
2010-08-01 18:57:02 | mark.dickinson | set | nosy:
+ mark.dickinson messages:
+ msg112367
|
2010-03-09 22:49:52 | Alexander.Belopolsky | set | messages:
+ msg100768 |
2010-03-09 22:15:02 | ronaldoussoren | set | messages:
+ msg100766 |
2010-03-09 22:14:41 | ronaldoussoren | set | files:
- smime.p7s |
2010-03-09 22:06:45 | Alexander.Belopolsky | set | messages:
+ msg100764 |
2010-03-08 22:24:10 | zvezdan | set | nosy:
+ zvezdan
|
2010-03-07 07:22:51 | ronaldoussoren | set | files:
+ smime.p7s
messages:
+ msg100566 |
2010-03-07 00:00:45 | Alexander.Belopolsky | set | nosy:
+ ronaldoussoren messages:
+ msg100553
|
2010-03-05 17:59:41 | Alexander.Belopolsky | set | files:
+ issue8065-tests.diff
messages:
+ msg100495 |
2010-03-05 17:06:38 | Alexander.Belopolsky | set | files:
+ issue8065.diff
nosy:
+ Alexander.Belopolsky messages:
+ msg100492
keywords:
+ patch |
2010-03-05 10:27:40 | stefanholek | create | |