Skip to content

bpo-34080: Fix memory leak on parsing error#8242

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
vstinner wants to merge 2 commits into from
Closed

bpo-34080: Fix memory leak on parsing error #8242

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6224531
bpo-34080: Fix memory leak on parsing error
vstinner Jul 11, 2018
fdf73c9
Only clear err->text in err_free()
vstinner Jul 11, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions Misc/NEWS.d/next/Core and Builtins/2018-07-11-11-20-12.bpo-34080.8t7PtO.rst
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
Fixed a memory leak in the compiler when it raised some uncommon errors
during tokenizing.
7 changes: 4 additions & 3 deletions Parser/parsetok.c
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -246,8 +246,6 @@ parsetok(struct tok_state *tok, grammar *g, int start, perrdetail *err_ret,
else if ((ps->p_flags & CO_FUTURE_BARRY_AS_BDFL) &&
strcmp(str, "<>")) {
PyObject_FREE(str);
err_ret->text = "with Barry as BDFL, use '<>' "
"instead of '!='";
err_ret->error = E_SYNTAX;
break;
}
Expand DownExpand Up@@ -322,7 +320,10 @@ parsetok(struct tok_state *tok, grammar *g, int start, perrdetail *err_ret,
assert(tok->cur - tok->buf < INT_MAX);
err_ret->offset = (int)(tok->cur - tok->buf);
len = tok->inp - tok->buf;
err_ret->text = (char *) PyObject_MALLOC(len + 1);
/* make sure that we don't leak memory if text has been
set previously */
assert(err_ret->text == NULL);
err_ret->text = (char *) PyObject_Malloc(len + 1);
if (err_ret->text != NULL) {
if (len > 0)
strncpy(err_ret->text, tok->buf, len);
Expand Down
10 changes: 4 additions & 6 deletions Python/pythonrun.c
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -63,7 +63,7 @@ static PyObject *run_mod(mod_ty, PyObject *, PyObject *, PyObject *,
PyCompilerFlags *, PyArena *);
static PyObject *run_pyc_file(FILE *, const char *, PyObject *, PyObject *,
PyCompilerFlags *);
static void err_input(perrdetail *);
static void err_input(const perrdetail *);
static void err_free(perrdetail *);
static int PyRun_InteractiveOneObjectEx(FILE *, PyObject *, PyCompilerFlags *);

Expand DownExpand Up@@ -1324,12 +1324,14 @@ static void
err_free(perrdetail *err)
{
Py_CLEAR(err->filename);
PyObject_Free(err->text);
err->text = NULL;
}

/* Set the error appropriate to the given input error code (see errcode.h) */

static void
err_input(perrdetail *err)
err_input(const perrdetail *err)
{
PyObject *v, *w, *errtype, *errtext;
PyObject *msg_obj = NULL;
Expand DownExpand Up@@ -1445,10 +1447,6 @@ err_input(perrdetail *err)
Py_XDECREF(w);
cleanup:

@serhiy-storchakaserhiy-storchakaJul 11, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

goto cleanup is used even in branches when msg_obj is always NULL. Either replace them with return, or merge #8222.

@vstinnervstinnerJul 11, 2018

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see anything wrong with calling Py_XDECREF(msg_obj) when msg_obj is NULL. It's a common try/finally-like pattern applied to the C language. Moreover, I wrote this PR to fix a real memory leak, not to cleanup the code. Don't hesitate to propose a new PR once the memory leak is fixed, if you want to clean up the code further. I prefer to write the shortest PR to be able to backport it to all branches.

Py_XDECREF(msg_obj);
if (err->text != NULL) {
PyObject_FREE(err->text);
err->text = NULL;
}
}


Expand Down