Skip to content
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.

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

Set kp_debug, kp_info, kp_warning and kp_error to py::none() when the program terminates. #231

Conversation

thinking-tower
Copy link
Contributor

cc @unexploredtest, @axsaucedo, Superchunk. This should fix #230.

Description

The first instance of segfault occurs with a0f9af9, where

// main.cpp
- py::object kp_logger;
+ py::object kp_debug, kp_info, kp_warning, kp_error;

I suspect that the "attributes" of the Logger class (in this case,

kp_debug    = kp_logger.attr("debug");
kp_info     = kp_logger.attr("info");
kp_warning  = kp_logger.attr("warning");
kp_error    = kp_logger.attr("error");

) are cleaned up once when the program terminates and then the static global py::objects are cleaned up again, leading to segfault.

In PyTorch, there is a similar idea where they use static variables to "cache" py::object attributes. Here is a short summary:

// python_rpc_handler.cpp
void PythonRpcHandler::init() {
    // ...
    py::object rpcInternal = py::module::import(kInternalModule);   
    // ...
    pyRunFunction_ = getFunction(rpcInternal, "_run_function");    
    pySerialize_ = getFunction(rpcInternal, "serialize");    
    pyDeserialize_ = getFunction(rpcInternal, "deserialize");
}

py::object getFunction(const py::object& module, const char* name) {  
    py::object fn = module.attr(name);  
    TORCH_CHECK(      
              py::isinstance<py::function>(fn),
              "attribute ",   
              name,      
              " is not a function"
    );
    return fn;
}

In this PR from PyTorch, the fix was to set them to py:none during cleanup.

The pybind docs suggests using the atexit for cleanup.

It's unclear why this wasn't an issue in python3.8 and below.

Minimal Reproducible Example

  • git clone https://github.com/pybind/python_example
  • Change src/main.cpp to
#include <pybind11/pybind11.h>

py::object kp_debug;

PYBIND11_MODULE(kp, m) {
    py::module_ logging  = py::module_::import("logging");
    py::object kp_logger = logging.attr("getLogger")("kp");
    kp_debug             = kp_logger.attr("debug");
    /* // Add me to fix!
    auto atexit = py::module_::import("atexit");
    atexit.attr("register")(py::cpp_function([](){
        kp_debug = py::none();
        kp_info = py::none();
        kp_warning = py::none();
        kp_error = py::none();
    }));
    */
}
  • python3.9 -m pip install .
  • python3.9 -c "import kp"

@axsaucedo axsaucedo self-requested a review June 12, 2021 10:44
@axsaucedo axsaucedo merged commit e77a684 into KomputeProject:master Jun 12, 2021
@unexploredtest
Copy link
Contributor

Neat! I used to have the same issue on my laptop and I can confirm that it fixed the problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Python segfaults after import kp
3 participants