Skip to content

opcode cache for LOAD_GLOBAL emits false alarm in memory leak hunting #81327

New issue
New issue
Closed
Closed
opcode cache for LOAD_GLOBAL emits false alarm in memory leak hunting#81327
Assignees
tiran
Labels
3.9 (EOL)end of lifeinterpreter-core(Objects, Python, Grammar, and Parser dirs)type-bugAn unexpected behavior, bug, or error

Description

@vstinner
vstinner
opened on Jun 3, 2019
BPO 37146
Nosy @gvanrossum, @vstinner, @methane, @1st1, @pablogsal
PRs
  • bpo-37146: disable opcache when Py_DEBUG is defined #13787
  • bpo-37146: Allow to configure opcode cache and fix huntleaks by presetting it to 1 #13789
  • bpo-42093: Cleanup _PyDict_GetItemHint() #24582
  • bpo-37146: Deactivate opcode cache only when using huntrleaks in the test suite #24643
  • bpo-37146: Move _PyEval_DeactivateOpCache() to the internal C API #24786
    		•

    Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = 'https://github.com/tiran'
closed_at = <Date 2021-02-28.22:42:42.130>
created_at = <Date 2019-06-03.20:34:43.896>
labels = ['interpreter-core', 'type-bug', '3.9']
title = 'opcode cache for LOAD_GLOBAL emits false alarm in memory leak hunting'
updated_at = <Date 2021-03-08.21:56:41.259>
user = 'https://github.com/vstinner'

    bugs.python.org fields:

    activity = <Date 2021-03-08.21:56:41.259>
actor = 'vstinner'
assignee = 'christian.heimes'
closed = True
closed_date = <Date 2021-02-28.22:42:42.130>
closer = 'pablogsal'
components = ['Interpreter Core']
creation = <Date 2019-06-03.20:34:43.896>
creator = 'vstinner'
dependencies = []
files = []
hgrepos = []
issue_num = 37146
keywords = ['patch']
message_count = 13.0
messages = ['344469', '344485', '344486', '353666', '353700', '387335', '387338', '387339', '387361', '387368', '387400', '387820', '388311']
nosy_count = 6.0
nosy_names = ['gvanrossum', 'vstinner', 'methane', 'yselivanov', 'pablogsal', 'webstarnantes']
pr_nums = ['13787', '13789', '24582', '24643', '24786']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue37146'
versions = ['Python 3.9']

    Activity

    1. vstinner commented on Jun 3, 2019

      @vstinner
      vstinner
      on Jun 3, 2019
      MemberAuthor

      opcode cache for LOAD_GLOBAL introduced false alarm in memory leak hunting (python3 -m test -R 3:3 ...).

      => opcache: bpo-26219.

      Before the change:

      $ git checkout 91234a16367b56ca03ee289f7c03a34d4cfec4c8^
$ make && ./python -m test -R 3:3 test_pprint 
...
Tests result: SUCCESS

      After the change:

      $ git checkout 91234a16367b56ca03ee289f7c03a34d4cfec4c8
$ make && ./python -m test -R 3:3 test_pprint 
...
test_pprint leaked [4, 2, 4] memory blocks, sum=10
...

      The problem is that at each iteration of regrtest -R 3:3 (6 iterations), a few more code objects get this opcache allocated.

      There are different solutions to fix regrtest -R 3:3.

      (*) Always optimize:

      diff --git a/Python/ceval.c b/Python/ceval.c
index 411ba3d73c..6cd148efba 100644
--- a/Python/ceval.c
+++ b/Python/ceval.c
@@ -103,7 +103,7 @@ static long dxp[256];
 #endif
 
 /* per opcode cache */
-#define OPCACHE_MIN_RUNS 1024  /* create opcache when code executed this time */
+#define OPCACHE_MIN_RUNS 1  /* create opcache when code executed this time */
 #define OPCACHE_STATS 0  /* Enable stats */
 
 #if OPCACHE_STATS
      $ make && ./python -m test -R 3:3 test_pprint 
...
Tests result: SUCCESS

      (*) Never optimmize: disable opcache until a better fix can be found

      diff --git a/Python/ceval.c b/Python/ceval.c
index 411ba3d73c..3c85df6fea 100644
--- a/Python/ceval.c
+++ b/Python/ceval.c
@@ -1230,6 +1230,7 @@ _PyEval_EvalFrameDefault(PyFrameObject *f, int throwflag)
     f->f_stacktop = NULL;       /* remains NULL unless yield suspends frame */
     f->f_executing = 1;
 
+#if 0
     if (co->co_opcache_flag < OPCACHE_MIN_RUNS) {
         co->co_opcache_flag++;
         if (co->co_opcache_flag == OPCACHE_MIN_RUNS) {
@@ -1244,6 +1245,7 @@ _PyEval_EvalFrameDefault(PyFrameObject *f, int throwflag)
 #endif
         }
     }
+#endif
 
 #ifdef LLTRACE
     lltrace = _PyDict_GetItemId(f->f_globals, &PyId___ltrace__) != NULL;
      $ make && ./python -m test -R 3:3 test_pprint 
...
Tests result: SUCCESS

      (*) Find a way to explicitly deoptimize all code objects

      Modules/gcmodule.c has a clear_freelists() function called by collect() if generation == NUM_GENERATIONS-1: on when gc.collect() is collected explicitly for example.

      Lib/test/libregrtest/refleak.py also has a dash_R_cleanup() function which clears many caches.

      Problem: currently, code objects are not explicitly tracked (for example, they are not tracked in a double linked list).

      (*) Add way more warmup iterations to regrtest in buildbots.

      I dislike this option. A build on a refleak buildbot worker already takes 2 to 3 hours. Adding more warmup would make a build even way more slower.

    2. vstinner
      added
      interpreter-core(Objects, Python, Grammar, and Parser dirs)
      3.8 (EOL)end of life
      on Jun 3, 2019

    3. vstinner commented on Jun 3, 2019

      @vstinner
      vstinner
      on Jun 3, 2019
      MemberAuthor

      New changeset eddef86 by Victor Stinner (Inada Naoki) in branch 'master':
      bpo-37146: disable opcache when Py_DEBUG is defined (GH-13787)
      eddef86

    4. vstinner commented on Jun 3, 2019

      @vstinner
      vstinner
      on Jun 3, 2019
      MemberAuthor

      INADA-san wrote PR 13787 to disable opcache when Python is compiled in debug mode. Pablo and me approved the change, so I merged it.

      Pablo wrote a more robust solution, PR 13789, to disable opcache only in regrtest, to look for memory leaks. But INADA-san had a good argument against this approach:
      #13789 (comment)

      "The code object will be optimized only when ++co->co_opcache_flag == opcacheminruns. So decreasing min_runs by _setopcacheminruns()will cause some hot codes will be not optimized forever. I don't want to expose such switch."

      I would prefer to keep this issue until a long term approach is designed.

    5. vstinner commented on Oct 1, 2019

      @vstinner
      vstinner
      on Oct 1, 2019
      MemberAuthor

      It seems like nobody came up with a solution for the debug mode since June. I close the issue. Reopen it if you would like to propose a solution.

    6. vstinner
      closed this as completedon Oct 1, 2019

    7. 1st1 commented on Oct 1, 2019

      @1st1
      1st1
      on Oct 1, 2019
      Member

      It seems like nobody came up with a solution for the debug mode since June. I close the issue. Reopen it if you would like to propose a solution.

      I think the only solution is to have a flag to disable optimizations, inlcluding this one.

    8. gvanrossum commented on Feb 19, 2021

      @gvanrossum
      gvanrossum
      on Feb 19, 2021
      Member

      Please revert this and use a separate build flag (e.g. DISABLE_INLINE_CACHES) for the refleaks run. (Or perhaps provide an -X flag to disable it without the need to recompile.)

      I am developing new inline cache ideas and of course I need to run in debug mode, so I regularly end up wasting some time until I remember to add a patch that changes OPCACHE_MIN_RUNS back.

    9. vstinner commented on Feb 19, 2021

      @vstinner
      vstinner
      on Feb 19, 2021
      MemberAuthor

      I reopen the issue (but I'm not interested by trying to fix it).

    10. vstinner
      reopened this on Feb 19, 2021

    11. vstinner commented on Feb 19, 2021

      @vstinner
      vstinner
      on Feb 19, 2021
      MemberAuthor

      (Or perhaps provide an -X flag to disable it without the need to recompile.)

      Giving the ability to control the cache size, at least at Python startup, is one option.

      Or maybe need a generic -X flag to tell Python that libregrtest is tracking leaks, since we might need to change other parameters than only the opcode cache size.

    12. 1st1 commented on Feb 19, 2021

      @1st1
      1st1
      on Feb 19, 2021
      Member

      Giving the ability to control the cache size, at least at Python startup, is one option.

      I'd really prefer not to allow users to control cache sizes. There's basically no point in that; the only practically useful thing is to enable or disable it.

    13. pablogsal commented on Feb 19, 2021

      @pablogsal
      pablogsal
      on Feb 19, 2021
      Member

      Or maybe need a generic -X flag to tell Python that libregrtest is tracking leaks, since we might need to change other parameters than only the opcode cache size.

      I think we should go this way, maybe even less "public". If we add a -X, then it will be too visible. Maybe some env variable?

    14. webstarnantes commented on Feb 20, 2021

      @webstarnantes
      webstarnantesmannequin
      on Feb 20, 2021
      Mannequin

      dans cette version pour securité maximale

      SSLContext.wrap_socket()
import socket
import ssl

hostname = 'www.python.org'
context = ssl.create_default_context()

with socket.create_connection((hostname, 443)) as sock:
    with context.wrap_socket(sock, server_hostname=hostname) as ssock:
        print(ssock.version())

      https://backlinkstrong.com/

    15. 1 remaining item

    16. webstarnantes
      added
      3.9 (EOL)end of life
      type-securityA security issue
      and removed
      interpreter-core(Objects, Python, Grammar, and Parser dirs)
      3.8 (EOL)end of life
      on Feb 20, 2021
    17. webstarnantes
      changed the title [-]opcode cache for LOAD_GLOBAL emits false alarm in memory leak hunting[/-] [+]SSl Securité version 3.9.2[/+] on Feb 20, 2021
    18. webstarnantes
      assigned
      tiran
      on Feb 20, 2021
    19. tiran
      added
      interpreter-core(Objects, Python, Grammar, and Parser dirs)
      and removed
      topic-SSL
      on Feb 20, 2021
    20. tiran
      changed the title [-]SSl Securité version 3.9.2[/-] [+]opcode cache for LOAD_GLOBAL emits false alarm in memory leak hunting[/+] on Feb 20, 2021
    21. tiran
      added
      type-bugAn unexpected behavior, bug, or error
      and removed
      type-securityA security issue
      on Feb 20, 2021

    22. pablogsal commented on Feb 28, 2021

      @pablogsal
      pablogsal
      on Feb 28, 2021
      Member

      New changeset af5fa13 by Pablo Galindo in branch 'master':
      bpo-37146: Deactivate opcode cache only when using huntrleaks in the test suite (GH-24643)
      af5fa13

    23. pablogsal
      closed this as completedon Feb 28, 2021

    24. vstinner commented on Mar 8, 2021

      @vstinner
      vstinner
      on Mar 8, 2021
      MemberAuthor

      New changeset 9f672a5 by Victor Stinner in branch 'master':
      bpo-37146: Move _PyEval_DeactivateOpCache() to the internal C API (GH-24786)
      9f672a5

    25. ezio-melotti
      transferred this issue fromon Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Metadata

    Metadata

    Assignees

    Labels

    3.9 (EOL)end of lifeinterpreter-core(Objects, Python, Grammar, and Parser dirs)type-bugAn unexpected behavior, bug, or error

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions