Where communities thrive


  • Join over 1.5M+ people
  • Join over 100K+ communities
  • Free without limits
  • Create your own community
People
Activity
  • Dec 10 17:01
    antonmilev opened #2021
  • Dec 09 18:51
    T045T commented #2019
  • Dec 09 18:23
    T045T synchronize #2019
  • Dec 09 16:41
    T045T synchronize #2019
  • Dec 09 16:32
    T045T opened #2019
  • Dec 08 21:50
    antonmilev commented #1763
  • Dec 08 20:08
    tarasko ready_for_review #2013
  • Dec 08 20:05
    tarasko edited #2013
  • Dec 08 19:46
    tarasko edited #2013
  • Dec 08 19:44
    tarasko synchronize #2013
  • Dec 07 23:22
    tarasko synchronize #2013
  • Dec 07 21:56
    tarasko synchronize #2013
  • Dec 07 21:49
    tarasko edited #2013
  • Dec 07 21:41
    tarasko synchronize #2013
  • Dec 07 21:30
    tarasko edited #2013
  • Dec 07 21:19
    tarasko synchronize #2013
  • Dec 07 20:42
    tarasko synchronize #2013
  • Dec 07 19:27
    tarasko edited #2013
  • Dec 07 17:27
    tarasko edited #2013
  • Dec 07 16:57
    tarasko edited #2013
Boris Staletic
@bstaletic
That worked.
Yannick Jadoul
@YannickJadoul
Ahaaa
Nice
Boris Staletic
@bstaletic
The possibly lost and still reachable are at an all time low with python 3.8.
Yannick Jadoul
@YannickJadoul
And what if the interpreter is running as main thing?
I'm guessing py::finalize_interpreter isn't called then, or is it?
Boris Staletic
@bstaletic
I'm not sure. How should I test that?
You can't call py::finalize_interpreter, because you aren't in charge of maintaining the interpreter.
The process that loaded the extension module is.
PYTHONMALLOC=malloc valgrind python -c 'import ycm_core' and the output was...
==14543==    definitely lost: 104 bytes in 1 blocks
==14543==    indirectly lost: 0 bytes in 0 blocks
==14543==      possibly lost: 260,043 bytes in 1,413 blocks
==14543==    still reachable: 566,855 bytes in 4,552 blocks
Yannick Jadoul
@YannickJadoul
Hmm, and that's the same "definitely lost" block?
Boris Staletic
@bstaletic
Nope. This one is 100 bytes larger.
==14848== 104 bytes in 1 blocks are definitely lost in loss record 1,846 of 2,687
==14848==    at 0x4838DEF: operator new(unsigned long) (vg_replace_malloc.c:344)
==14848==    by 0x5A387C1: PyInit_ycm_core (in /home/bstaletic/work/ycmd/ycm_core.so)
==14848==    by 0x4C4FB89: _PyImport_LoadDynamicModuleWithSpec (in /usr/lib/libpython3.8.so.1.0)
==14848==    by 0x4C4FD69: ??? (in /usr/lib/libpython3.8.so.1.0)
==14848==    by 0x4B5E8C6: ??? (in /usr/lib/libpython3.8.so.1.0)
==14848==    by 0x4B6DE90: PyVectorcall_Call (in /usr/lib/libpython3.8.so.1.0)
==14848==    by 0x4BC2ADF: _PyEval_EvalFrameDefault (in /usr/lib/libpython3.8.so.1.0)
==14848==    by 0x4B8D039: _PyEval_EvalCodeWithName (in /usr/lib/libpython3.8.so.1.0)
==14848==    by 0x4B8DDEE: _PyFunction_Vectorcall (in /usr/lib/libpython3.8.so.1.0)
==14848==    by 0x4BC197C: _PyEval_EvalFrameDefault (in /usr/lib/libpython3.8.so.1.0)
==14848==    by 0x4B8DD5A: _PyFunction_Vectorcall (in /usr/lib/libpython3.8.so.1.0)
==14848==    by 0x4BBD88E: _PyEval_EvalFrameDefault (in /usr/lib/libpython3.8.so.1.0)
Yannick Jadoul
@YannickJadoul
Huh
Ah, maybe also the whole "internals" block?
Boris Staletic
@bstaletic
Maybe... It's not CPython itself, because importing _io instead works.
Yannick Jadoul
@YannickJadoul
Is there a C API alternative for this: https://docs.python.org/3/library/atexit.html
At most 32 cleanup functions can be registered.
Jikes

Since Python’s internal finalization will have completed before the cleanup function, no Python APIs should be called by func.

OK, not possible anyway, I guess

Boris Staletic
@bstaletic
Because at most CPU registers have 5 bits and python doesn't like to waste cycles?
Yannick Jadoul
@YannickJadoul
"python doesn't like to waste cycles?" :-D Haha, good one
Ah, but it's going to be hard to do this. Because delete *internals_ptr_ptr; needs to happen after the finalization (Py_AtExit would work, I guess?)
Boris Staletic
@bstaletic
Currently my finalize_interpreter() looks like this:
Yannick Jadoul
@YannickJadoul
But to free the TLS, maybe a capsule? Though that seems overkill for the few bytes
Boris Staletic
@bstaletic
inline void finalize_interpreter() {
    handle builtins(PyEval_GetBuiltins());
    const char *id = PYBIND11_INTERNALS_ID;

    // Get the internals pointer (without creating it if it doesn't exist).  It's possible for the
    // internals to be created during Py_Finalize() (e.g. if a py::capsule calls `get_internals()`
    // during destruction), so we get the pointer-pointer here and check it after Py_Finalize().
    detail::internals **internals_ptr_ptr = detail::get_internals_pp();
    // It could also be stashed in builtins, so look there too:
    if (builtins.contains(id) && isinstance<capsule>(builtins[id]))
        internals_ptr_ptr = capsule(builtins[id]);
    if (internals_ptr_ptr && *internals_ptr_ptr)
        PyThread_tss_free( (*internals_ptr_ptr)->tstate );
    Py_Finalize();

    if (internals_ptr_ptr) {
        delete *internals_ptr_ptr;
        *internals_ptr_ptr = nullptr;
    }
}
Would going back to the ~internals() be a better idea?
Yannick Jadoul
@YannickJadoul
Yeah, so ... I'm guessing Py_Finalize will garbage collect all objects? So maybe a "lost" capsule that calls PyThread_tss_free would work?

Would going back to the ~internals() be a better idea?

No. By then it's (maybe) too late to call PyThread_tss_free, after Py_Finalize?

Boris Staletic
@bstaletic
Could be.
This is why languages need to be designed with destructors in mind.
Yannick Jadoul
@YannickJadoul
Hard to do with a garbage collecting language, though?
Boris Staletic
@bstaletic
Yes, I have just said that ~99% of programming languages are broken.
Boris Staletic
@bstaletic
@YannickJadoul Those 104 bytes leaked aren't internals. sizeof(internals) == 216.
Boris Staletic
@bstaletic
That pull request actually solves the leak.
Boris Staletic
@bstaletic
What does this mean?
Warning, treated as error:
/home/travis/build/pybind/pybind11/docs/reference.rst:60:Invalid definition: Expected end of definition. [error at 25]
  bool arg::flag_noconvert :  1
  -------------------------^
Also this:
==> Checking for dependents of upgraded formulae...
Error: undefined method `any?' for nil:NilClass
Is CI broken?
Yannick Jadoul
@YannickJadoul
@bstaletic Huh, that's weird. But ok, even better if it's solved now
Btw. What about adding a macros (as for the others TLS things) and making it a no-op before 3.7
Then we don't need to replicate this #if PY_VERSION_HEX >= 0x03070000 logic?
Boris Staletic
@bstaletic

@YannickJadoul Neither of those two errors are fixed.

As for the macro, consider the point of use.

if (cond)
    PyThread_tss_free(tstate);

If we make PyThread_tss_free a macro like this:

#if PY_VERSION_HEX >= 0x03070000
#define PYBIND_THREAD_FREE(thread_state) PyThread_tss_free(thread_state)
#else
#define PYBIND_THREAD_FREE
#endif

then the #else branch doesn't work.

Any better ideas?
Yannick Jadoul
@YannickJadoul
#define PYBIND_THREAD_FREE(ignored)?
Oh, right, wait
#define PYBIND_THREAD_FREE(key) (void) key
My main point is that there is already this whole thing in internals.h abstracting away this logic of choosing between pre-3.7 and post-3.7, so maybe you can just add it to those macros?