Internal C API: pass the memory allocator in a context #79446

Activity

1. vstinner commented on Nov 16, 2018

   

   vstinner

   on Nov 16, 2018

   MemberAuthor

   More actions

   Currently, the Python initialization code base is full of:

   static void
   pymain_clear_cmdline(_PyMain *pymain, _PyCmdline *cmdline)
   {
       PyMemAllocatorEx old_alloc;
       _PyMem_SetDefaultAllocator(PYMEM_DOMAIN_RAW, &old_alloc);
       ...
       PyMem_SetAllocator(PYMEM_DOMAIN_RAW, &old_alloc);
   }

   It's a hack that I wrote when I worked on the Python initialization code, to make sure that we use the same memory allocator to allocate and release memory. The problem is that Python allocates memory at startup, then the memory allocator can be changed in Py_Initialize() (ex: by PYTHONMALLOC environment variable), but at exit, we need to reuse the same memory allocator that we used to allocate memory early.

   Instead of "forcing" the memory allocator to "default allocator", I propose to add a new "context" structure which contains the memory allocator (PyMemAllocatorEx structure). Example:

   typedef struct {
       /* Enable UTF-8 mode?
          Set by -X utf8 command line option and PYTHONUTF8 environment variable.
          If set to -1 (default), inherit Py_UTF8Mode value. */
       int utf8_mode;

   PyMemAllocatorEx raw_alloc;
   

   } _PyConfigCtx;

   The context should be passed to any function allocating directly or indirectly memory.

   Py_Main(), "core config" and "path config" are good target for these changes. These functions are used during Python initialization.

   I know that Eric Snow would like to pass a "context" to functions of the C API. I don't want to make a giant change of the C API. I only want to modify the internal C API, and only modifiy Python internal. The context that I propose is designed for early Python initialization, and fix the practical problem of memory allocator.

   Later we can discussed how _PyRuntime and the pointer to the current interpreter can be added into this context, or if a new context should be created.

   Attached PR is a proof-of-concept of my idea, but the giant PR might be splitted into smaller changes to more incremental changes.
2. vstinner

   added

   3.8 (EOL)end of lifeend of life

   interpreter-core(Objects, Python, Grammar, and Parser dirs)(Objects, Python, Grammar, and Parser dirs)

   type-featureA feature request or enhancementA feature request or enhancement

   on Nov 16, 2018
3. vstinner commented on Nov 17, 2018

   

   vstinner

   on Nov 17, 2018

   MemberAuthor

   More actions

   Passing the memory allocator is my first goal.

   My second goal is to cleanup the code reading the configuration, pymain_read_conf() in Modules/main.c:

           int utf8_mode = config->ctx.utf8_mode;
           int encoding_changed = 0;

       (...)
   

           /* bpo-34207: Py_DecodeLocale() and Py_EncodeLocale() depend
              on Py_UTF8Mode and Py_LegacyWindowsFSEncodingFlag. */
           Py_UTF8Mode = config->ctx.utf8_mode;
   #ifdef MS_WINDOWS
           Py_LegacyWindowsFSEncodingFlag = config->legacy_windows_fs_encoding;
   #endif

       (...)
   

           /* Reset the configuration before reading again the configuration,
              just keep UTF-8 Mode value. */
           int new_utf8_mode = config->ctx.utf8_mode;
           int new_coerce_c_locale = config->coerce_c_locale;
           if (_PyCoreConfig_Copy(config, &save_config) < 0) {
               pymain->err = _Py_INIT_NO_MEMORY();
               goto done;
           }
           pymain_clear_cmdline(pymain, cmdline);
           config->ctx.utf8_mode = new_utf8_mode;
           config->coerce_c_locale = new_coerce_c_locale;

       /* The encoding changed: read again the configuration
          with the new encoding */
   

   My main concern is: "bpo-34207: Py_DecodeLocale() and Py_EncodeLocale() depend on Py_UTF8Mode and Py_LegacyWindowsFSEncodingFlag". Python initialization code should depend on global variables.
4. ncoghlan commented on Nov 18, 2018

   

   ncoghlan

   on Nov 18, 2018

   Contributor

   More actions

   I think the idea makes sense, but find the proposed name potentially confusing for two reasons:

   1. It isn't only about configuration, it's about interpreter initialisation state in general
   2. We use "context" for several other purposes already (most notably with statement context managers and async contexts)

   Given the proposed "_PyPreConfig" construct, how would you feel about calling this "_PyPreInitState"? The idea there being that like PreConfig, the PreInitState would be discarded once the interpreter was fully initialised - the PreInitState would only be needed to handle cleanup *while* initialising (either of temporary values, or in the event of initialisation failing and needing to undo previous allocations).

   My comment from bpo-32566 also applies here: PEP-432 is now lagging so far behind the state of the private API implementation that it really needs an update to better reflect where you're wanting to take the public initialisation API following these changes.
5. vstinner commented on Nov 22, 2018

   

   vstinner

   on Nov 22, 2018

   MemberAuthor

   More actions

   Honestly, I'm not convinced that the change is worth it. It looks like a giant complication for a very specific use case, whereas the current _PyMem_SetDefaultAllocator() solution "works". I close the issue.

   Moreover, bpo-35266 conflicts with issue, and bpo-35266 looks more important than this issue, since it impacts the future public API and remove redundancy.
6. vstinner

   closed this as completedon Nov 22, 2018
7. ezio-melotti

   transferred this issue fromon Apr 10, 2022