Skip to content

Conversation

@serhiy-storchaka
Copy link
Member

@serhiy-storchaka serhiy-storchaka commented Jul 10, 2018

@zhangyangyu
Copy link
Member

In parsetok.c, err_ret->text is assigned a not malloced string, with Barry as BDFL ..., does it matter?

@serhiy-storchaka
Copy link
Member Author

Interesting. This looks as a bug, but it is not related to this PR, because it sets err_ret->error = E_SYNTAX, while this PR fixes a case for err_ret->error = E_ERROR. Please open a separate issue for that bug.

Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

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

IHMO err_input() is fine, it shouldn't clear err->msg if err->error == E_ERROR. I would suggest to fix err_clear() instead:

diff --git a/Python/pythonrun.c b/Python/pythonrun.c
index bcd1ca931d..1377be3cd5 100644
--- a/Python/pythonrun.c
+++ b/Python/pythonrun.c
@@ -1324,6 +1324,10 @@ static void
 err_free(perrdetail *err)
 {
     Py_CLEAR(err->filename);
+    if (err->text != NULL) {
+        PyObject_FREE(err->text);
+        err->text = NULL;
+    }
 }
 
 /* Set the error appropriate to the given input error code (see errcode.h) */

@bedevere-bot
Copy link

When you're done making the requested changes, leave the comment: I have made the requested changes; please review again.

@vstinner
Copy link
Member

I would suggest to fix err_clear() instead: (...)

FYI "./python -m test -j0 -R 3:3 test_compile test_grammar" pass with this change, but then the following issue should be addressed as well?

"In parsetok.c, err_ret->text is assigned a not malloced string, with Barry as BDFL ..., does it matter?"

@zhangyangyu
Copy link
Member

zhangyangyu commented Jul 11, 2018

err_input is only called when the parsing operation fails. err_free is always called no matter the operation succeeds or fails. err->text has value only when error occurs while err->filename is always initialized. That may be why the cleaning logic is put in err_input. Anyway, no matter where it is put, I think we can remove the NULL test since PyObject_FREE can handle it itself.

@vstinner It's issue34084. Add you to the nosy list.

@vstinner
Copy link
Member

I wrote PR #8242 to implement my proposed fix and fix also the issue spotted by @zhangyangyu.

@miss-islington
Copy link
Contributor

Thanks @serhiy-storchaka for the PR, and @vstinner for merging it 🌮🎉.. I'm working now to backport this PR to: 2.7, 3.6, 3.7.
🐍🍒⛏🤖

@bedevere-bot
Copy link

GH-8256 is a backport of this pull request to the 3.7 branch.

@bedevere-bot
Copy link

GH-8257 is a backport of this pull request to the 3.6 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jul 11, 2018
(cherry picked from commit 993030a)

Co-authored-by: Serhiy Storchaka <[email protected]>
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jul 11, 2018
(cherry picked from commit 993030a)

Co-authored-by: Serhiy Storchaka <[email protected]>
@bedevere-bot
Copy link

GH-8258 is a backport of this pull request to the 2.7 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jul 11, 2018
(cherry picked from commit 993030a)

Co-authored-by: Serhiy Storchaka <[email protected]>
miss-islington added a commit that referenced this pull request Jul 11, 2018
(cherry picked from commit 993030a)

Co-authored-by: Serhiy Storchaka <[email protected]>
miss-islington added a commit that referenced this pull request Jul 11, 2018
(cherry picked from commit 993030a)

Co-authored-by: Serhiy Storchaka <[email protected]>
vstinner pushed a commit that referenced this pull request Jul 11, 2018
(cherry picked from commit 993030a)

Co-authored-by: Serhiy Storchaka <[email protected]>
CuriousLearner added a commit to CuriousLearner/cpython that referenced this pull request Jul 14, 2018
* master: (2633 commits)
  bpo-34087: Fix buffer overflow in int(s) and similar functions (pythonGH-8274)
  bpo-34108: Fix double carriage return in 2to3 on Windows (python#8271)
  bpo-4260: Document that ctypes.xFUNCTYPE are decorators (pythonGH-7924)
  bpo-33723: Fix test_time.test_thread_time() (pythonGH-8267)
  bpo-33967: Remove use of deprecated assertRaisesRegexp() (pythonGH-8261)
  bpo-34080: Fix a memory leak in the compiler. (pythonGH-8222)
  Enable GUI testing on Travis Linux builds via Xvfb (pythonGH-7887)
  bpo-23927: Make getargs.c skipitem() skipping 'w*'. (pythonGH-8192)
  bpo-33648: Remove PY_WARN_ON_C_LOCALE (pythonGH-7114)
  bpo-34092, test_logging: increase SMTPHandlerTest timeout (pythonGH-8245)
  Simplify __all__ in multiprocessing (pythonGH-6856)
  bpo-34083: Update dict order in Functional HOWTO (pythonGH-8230)
  Doc: Point to Simple statements section instead of PEP (pythonGH-8238)
  bpo-29442: Replace optparse with argparse in setup.py (pythonGH-139)
  bpo-33597: Add What's New for PyGC_Head (pythonGH-8236)
  Dataclasses: Fix example on 30.6.8, add method should receive a list rather than an integer. (pythonGH-8038)
  Fix documentation for input and output tutorial (pythonGH-8231)
  bpo-34009: Expand on platform support changes (pythonGH-8022)
  Factor-out two substantially identical code blocks. (pythonGH-8219)
  bpo-34031: fix incorrect usage of self.fail in two tests (pythonGH-8091)
  ...
CuriousLearner added a commit to CuriousLearner/cpython that referenced this pull request Jul 15, 2018
* master: (1159 commits)
  bpo-34087: Fix buffer overflow in int(s) and similar functions (pythonGH-8274)
  bpo-34108: Fix double carriage return in 2to3 on Windows (python#8271)
  bpo-4260: Document that ctypes.xFUNCTYPE are decorators (pythonGH-7924)
  bpo-33723: Fix test_time.test_thread_time() (pythonGH-8267)
  bpo-33967: Remove use of deprecated assertRaisesRegexp() (pythonGH-8261)
  bpo-34080: Fix a memory leak in the compiler. (pythonGH-8222)
  Enable GUI testing on Travis Linux builds via Xvfb (pythonGH-7887)
  bpo-23927: Make getargs.c skipitem() skipping 'w*'. (pythonGH-8192)
  bpo-33648: Remove PY_WARN_ON_C_LOCALE (pythonGH-7114)
  bpo-34092, test_logging: increase SMTPHandlerTest timeout (pythonGH-8245)
  Simplify __all__ in multiprocessing (pythonGH-6856)
  bpo-34083: Update dict order in Functional HOWTO (pythonGH-8230)
  Doc: Point to Simple statements section instead of PEP (pythonGH-8238)
  bpo-29442: Replace optparse with argparse in setup.py (pythonGH-139)
  bpo-33597: Add What's New for PyGC_Head (pythonGH-8236)
  Dataclasses: Fix example on 30.6.8, add method should receive a list rather than an integer. (pythonGH-8038)
  Fix documentation for input and output tutorial (pythonGH-8231)
  bpo-34009: Expand on platform support changes (pythonGH-8022)
  Factor-out two substantially identical code blocks. (pythonGH-8219)
  bpo-34031: fix incorrect usage of self.fail in two tests (pythonGH-8091)
  ...
CuriousLearner added a commit to CuriousLearner/cpython that referenced this pull request Jul 21, 2018
…ssue-33014

* 'master' of github.com:CuriousLearner/cpython: (722 commits)
  bpo-34087: Fix buffer overflow in int(s) and similar functions (pythonGH-8274)
  bpo-34108: Fix double carriage return in 2to3 on Windows (python#8271)
  bpo-4260: Document that ctypes.xFUNCTYPE are decorators (pythonGH-7924)
  bpo-33723: Fix test_time.test_thread_time() (pythonGH-8267)
  bpo-33967: Remove use of deprecated assertRaisesRegexp() (pythonGH-8261)
  bpo-34080: Fix a memory leak in the compiler. (pythonGH-8222)
  Enable GUI testing on Travis Linux builds via Xvfb (pythonGH-7887)
  bpo-23927: Make getargs.c skipitem() skipping 'w*'. (pythonGH-8192)
  bpo-33648: Remove PY_WARN_ON_C_LOCALE (pythonGH-7114)
  bpo-34092, test_logging: increase SMTPHandlerTest timeout (pythonGH-8245)
  Simplify __all__ in multiprocessing (pythonGH-6856)
  bpo-34083: Update dict order in Functional HOWTO (pythonGH-8230)
  Doc: Point to Simple statements section instead of PEP (pythonGH-8238)
  bpo-29442: Replace optparse with argparse in setup.py (pythonGH-139)
  bpo-33597: Add What's New for PyGC_Head (pythonGH-8236)
  Dataclasses: Fix example on 30.6.8, add method should receive a list rather than an integer. (pythonGH-8038)
  Fix documentation for input and output tutorial (pythonGH-8231)
  bpo-34009: Expand on platform support changes (pythonGH-8022)
  Factor-out two substantially identical code blocks. (pythonGH-8219)
  bpo-34031: fix incorrect usage of self.fail in two tests (pythonGH-8091)
  ...
@serhiy-storchaka serhiy-storchaka deleted the tokenizer-error-memory-leak branch November 6, 2018 06:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type-bug An unexpected behavior, bug, or error

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants