mamba icon indicating copy to clipboard operation
mamba copied to clipboard

v2: Ctrl+C / SIGINT / KeyboardInterrupt handling not working

Open jaimergp opened this issue 2 months ago • 4 comments

Troubleshooting docs

  • [X] My problem is not solved in the Troubleshooting docs

Anaconda default channels

  • [X] I do NOT use the Anaconda default channels (pkgs/* etc.)

How did you install Mamba?

Other (please describe)

Search tried in issue tracker

ctrl c handling

Latest version of Mamba

  • [X] My problem is not solved with the latest version

Tried in Conda?

Not applicable

Describe your issue

In the latest beta, Ctrl+C handling from Python doesn't work correctly. In v1, we added Context.use_default_signal_handler(...) as a workaround, but this code doesn't seem to work in v2 anymore.

This simple script reproduces the problem. Run it with python and try to Ctrl+C out of it:

import time
from libmambapy import Context
# This line below doesn't have any effect;
# it can be commented out and the result is the same
Context.use_default_signal_handler(False)
print("Sleep for 10s... Try to Ctrl+C")
time.sleep(10)

It just gets stuck until the end of the 10s sleep:

$ python debug_ctrl_c.py 
Sleep for 10s... Try to Ctrl+C
^C^C^C^C^C^C^C^C

If you remove the libmambapy imports, the outcome is:

$ python debug_ctrl_c.py 
Sleep for 10s... Try to Ctrl+C
^CTraceback (most recent call last):
  File "/workspaces/conda-libmamba-solver/debug_ctrl_c.py", line 6, in <module>
    time.sleep(10)
KeyboardInterrupt

mamba info / micromamba info

libmamba version : 2.0.0
          mamba version : 2.0.0
           curl version : libcurl/8.7.1 OpenSSL/3.2.1 zlib/1.2.13 zstd/1.5.5 libssh2/1.11.0 nghttp2/1.58.0
     libarchive version : libarchive 3.7.2 zlib/1.2.13 liblzma/5.2.6 bz2lib/1.0.8 liblz4/1.9.3 libzstd/1.5.5
       envs directories : /root/.local/share/mamba/envs
          package cache : /root/.local/share/mamba/pkgs
                          /root/.mamba/pkgs
            environment : /opt/conda (active)
           env location : /opt/conda
      user config files : /root/.mambarc
 populated config files : /opt/conda/.condarc
       virtual packages : __unix=0=0
                          __linux=5.15.49=0
                          __glibc=2.31=0
                          __archspec=1=aarch64
               channels : https://conda.anaconda.org/conda-forge/linux-aarch64
                          https://conda.anaconda.org/conda-forge/noarch
       base environment : /root/.local/share/mamba
               platform : linux-aarch64

Logs

No response

environment.yml

N/A

~/.condarc

N/A

jaimergp avatar Apr 12 '24 16:04 jaimergp

Is this issue also happening with alpha4 version? or just beta?

Hind-M avatar Apr 15 '24 09:04 Hind-M

~~You are right, this works correctly on alpha4. At least on Linux :)~~

Sorry, I tested the wrong file. I can reproduce the original report with alpha4 too. IOW, Ctrl+C doesn't work there either.

jaimergp avatar Apr 15 '24 14:04 jaimergp

Thanks for #3285 and the beta1 release. Unfortunately, it still doesn't fix the reproducer script for me. IOW, this still gets hung:

import time
from libmambapy import Context
# This line below doesn't have any effect;
# it can be commented out and the result is the same
Context.use_default_signal_handler(False)
print("Sleep for 10s... Try to Ctrl+C")
time.sleep(10)

With versions:

libmamba                  2.0.0beta1           h19be64c_0    conda-forge/label/mamba_dev
libmambapy                2.0.0beta1      py311h1564048_0    conda-forge/label/mamba_dev

Is that how I am supposed to use it? 🤔

Additionally, is #3285 accounting for Windows too?

jaimergp avatar May 07 '24 08:05 jaimergp

Yes that's the way to use it, so it is still broken :s

JohanMabille avatar May 07 '24 11:05 JohanMabille

I'm a bit surprised as it was working well for me but now I see that it only works when doing Ctrl+C twice. Not sure why, but this hotfix was part of a bigger fix so probably I didnt integrate something from the other fix (which was not complete yet). I'm on it.

Klaim avatar May 15 '24 09:05 Klaim

Correction: I was trying the wrong branch. The branch from the pr seems to work for me, one Ctrl+C does break the sleep. (I was surprised before because I used the same test script).

I also tested on main and it still works.

@jaimergp Looks like something is different between our testing cases, could you give more info about your setup so that I can attempt to reproduce it? To clarify: I'm testing on Windows right now.

Klaim avatar May 15 '24 10:05 Klaim

I'm on macOS and also tested on Linux via Docker:

$ conda create -n debug-libmamba-v2b1-ctrl-c --override-channels -c conda-forge/label/mamba_dev -c conda-forge python=3.10 libmambapy
Channels:
 - conda-forge/label/mamba_dev
 - conda-forge
Platform: osx-arm64
Collecting package metadata (repodata.json): done
Solving environment: done


==> WARNING: A newer version of conda exists. <==
    current version: 24.1.1
    latest version: 24.5.0

Please update conda by running

    $ conda update -n base -c conda-forge conda



## Package Plan ##

  environment location: /Users/jrodriguez/.local/anaconda/envs/debug-libmamba-v2b1-ctrl-c

  added / updated specs:
    - libmambapy
    - python=3.10


The following packages will be downloaded:

    package                    |            build
    ---------------------------|-----------------
    libmambapy-2.0.0beta1      |  py310h7ae444f_0         527 KB  conda-forge/label/mamba_dev
    ------------------------------------------------------------
                                           Total:         527 KB

The following NEW packages will be INSTALLED:

  bzip2              conda-forge/osx-arm64::bzip2-1.0.8-h93a5062_5 
  c-ares             conda-forge/osx-arm64::c-ares-1.28.1-h93a5062_0 
  ca-certificates    conda-forge/osx-arm64::ca-certificates-2024.2.2-hf0a4a13_0 
  cpp-expected       conda-forge/osx-arm64::cpp-expected-1.1.0-hffc8910_0 
  fmt                conda-forge/osx-arm64::fmt-10.2.1-h2ffa867_0 
  icu                conda-forge/osx-arm64::icu-73.2-hc8870d7_0 
  krb5               conda-forge/osx-arm64::krb5-1.21.2-h92f50d5_0 
  libarchive         conda-forge/osx-arm64::libarchive-3.7.2-hcacb583_1 
  libcurl            conda-forge/osx-arm64::libcurl-8.7.1-h2d989ff_0 
  libcxx             conda-forge/osx-arm64::libcxx-17.0.6-h5f092b4_0 
  libedit            conda-forge/osx-arm64::libedit-3.1.20191231-hc8eb9b7_2 
  libev              conda-forge/osx-arm64::libev-4.33-h93a5062_2 
  libffi             conda-forge/osx-arm64::libffi-3.4.2-h3422bc3_5 
  libiconv           conda-forge/osx-arm64::libiconv-1.17-h0d3ecfb_2 
  libmamba           conda-forge/label/mamba_dev/osx-arm64::libmamba-2.0.0beta1-h2db0620_0 
  libmambapy         conda-forge/label/mamba_dev/osx-arm64::libmambapy-2.0.0beta1-py310h7ae444f_0 
  libnghttp2         conda-forge/osx-arm64::libnghttp2-1.58.0-ha4dd798_1 
  libsolv            conda-forge/osx-arm64::libsolv-0.7.29-h1efcc80_0 
  libsqlite          conda-forge/osx-arm64::libsqlite-3.45.3-h091b4b1_0 
  libssh2            conda-forge/osx-arm64::libssh2-1.11.0-h7a5bd25_0 
  libxml2            conda-forge/osx-arm64::libxml2-2.12.7-ha661575_0 
  libzlib            conda-forge/osx-arm64::libzlib-1.2.13-h53f4e23_5 
  lz4-c              conda-forge/osx-arm64::lz4-c-1.9.4-hb7217d7_0 
  lzo                conda-forge/osx-arm64::lzo-2.10-h93a5062_1001 
  ncurses            conda-forge/osx-arm64::ncurses-6.5-hb89a1cb_0 
  nlohmann_json      conda-forge/osx-arm64::nlohmann_json-3.11.3-hebf3989_0 
  openssl            conda-forge/osx-arm64::openssl-3.3.0-h0d3ecfb_0 
  pybind11-abi       conda-forge/noarch::pybind11-abi-4-hd8ed1ab_3 
  python             conda-forge/osx-arm64::python-3.10.14-h2469fbe_0_cpython 
  python_abi         conda-forge/osx-arm64::python_abi-3.10-4_cp310 
  readline           conda-forge/osx-arm64::readline-8.2-h92ec313_1 
  reproc             conda-forge/osx-arm64::reproc-14.2.4.post0-h93a5062_1 
  reproc-cpp         conda-forge/osx-arm64::reproc-cpp-14.2.4.post0-h965bd2d_1 
  simdjson           conda-forge/osx-arm64::simdjson-3.9.2-h420ef59_0 
  spdlog             conda-forge/osx-arm64::spdlog-1.12.0-he64bfa9_2 
  tk                 conda-forge/osx-arm64::tk-8.6.13-h5083fa2_1 
  tzdata             conda-forge/noarch::tzdata-2024a-h0c530f3_0 
  xz                 conda-forge/osx-arm64::xz-5.2.6-h57fd34a_0 
  yaml-cpp           conda-forge/osx-arm64::yaml-cpp-0.8.0-h13dd4ca_0 
  zstd               conda-forge/osx-arm64::zstd-1.5.6-hb46c0d2_0 


Proceed ([y]/n)? y


Downloading and Extracting Packages:
                                                                                                                                                                                                                          
Preparing transaction: done
Verifying transaction: done
Executing transaction: done
#
# To activate this environment, use
#
#     $ conda activate debug-libmamba-v2b1-ctrl-c
#
# To deactivate an active environment, use
#
#     $ conda deactivate

$ conda activate debug-libmamba-v2b1-ctrl-c           
$ python ~/devel/conda-libmamba-solver/debug_ctrl_c.py
libmambapy version 2.0.0
Sleep for 5s... Try to Ctrl+C
^C^C^C^C^C^C^C^C^C^C^C%                                                                                              
$ conda info
     active environment : debug-libmamba-v2b1-ctrl-c
    active env location : /Users/jrodriguez/.local/anaconda/envs/debug-libmamba-v2b1-ctrl-c
            shell level : 2
       user config file : /Users/jrodriguez/.condarc
 populated config files : /Users/jrodriguez/.local/anaconda/.condarc
                          /Users/jrodriguez/.condarc
          conda version : 24.1.1
    conda-build version : 24.1.2
         python version : 3.9.9.final.0
                 solver : libmamba (default)
       virtual packages : __archspec=1=m1
                          __conda=24.1.1=0
                          __osx=14.3.1=0
                          __unix=0=0
       base environment : /Users/jrodriguez/.local/anaconda  (writable)
      conda av data dir : /Users/jrodriguez/.local/anaconda/etc/conda
  conda av metadata url : None
           channel URLs : https://conda.anaconda.org/conda-forge/osx-arm64
                          https://conda.anaconda.org/conda-forge/noarch
          package cache : /Users/jrodriguez/.local/anaconda/pkgs
                          /Users/jrodriguez/.conda/pkgs
       envs directories : /Users/jrodriguez/.local/anaconda/envs
                          /Users/jrodriguez/.conda/envs
               platform : osx-arm64
             user-agent : conda/24.1.1 requests/2.31.0 CPython/3.9.9 Darwin/23.3.0 OSX/14.3.1 solver/libmamba conda-libmamba-solver/24.1.0 libmambapy/1.5.6
                UID:GID : 501:20
             netrc file : None
           offline mode : False

I haven't tested the beta1 on Windows, but what I observed there during the early testing is that the process is finished after a couple of Ctrl+C, but abruptly. The correct behavior is that Python catches the signal and raises KeyboardInterrupt, which should appear in the output.

jaimergp avatar May 15 '24 10:05 jaimergp

It's possible that the hotfix is replacing a handler introduced by python, I'll try to do that better.

Klaim avatar May 15 '24 12:05 Klaim

Is it possible to have the C++ signal handler be opt-in via the application layer? IOW, that micromamba sets the handler explicitly, but that otherwise there's no signal specific logic set in libmamba(py)?

jaimergp avatar May 15 '24 14:05 jaimergp

Currently using libmamba in C++ will not set the handler by default and the option can be set through the Context constructor, see:

As you can see the signal handling is not set by default, but this is set to true in mamba and micromamba.

The issue here, indeed, is that in libmambapy we didnt want to break the way you use the library in python, so we had to default true for that option to keep the previous behavior (note that the issue here became apparent after the Context became not-a-singleton). The rigth solution to avoid this would be to change the python API so that the context has to be manually constructed with options exactly like in C++. We planned to do that in a future release though.

Meanwhile I'm trying to fix the issue without changing that API for the immediate time, although if you think you can wait for a future version I can stop and focus on the bigger better solution but it will only be available in a future minor release. As for the solution I'm trying: keep track of the previous handler that had been set and reuse it either when we set the option to false and/or in the handler we set.

Klaim avatar May 15 '24 14:05 Klaim

Thanks for the details!

v2 contains plenty of API breaking changes already so one more would be acceptable and not a problem. If you'd rather go that way, I'm fine waiting for that and sticking to v1 for the time being. Actually I think we are not explicitly initializing the context in https://github.com/conda/conda-libmamba-solver/pull/457/ anymore because we explicitly configure the Database and Solver with all the options (we do take the default values from conda's context, not libmamba's).

That said, if the interim solution is within acceptable reach (it doesn't take you hours of work), I'll be happy to assist with the debugging <3

As an aside question: should we be initializing the libmamba context ourselves? I was surprised to see that I still needed to set the context bool for the signal handlers, so I don't know if it's still being used behind the scenes for something else.

jaimergp avatar May 15 '24 14:05 jaimergp

That said, if the interim solution is within acceptable reach (it doesn't take you hours of work), I'll be happy to assist with the debugging <3

I'll see what we can do, although dont hold your breath for v2.0 because I suspect this change might also break a lot of mamba python tests .

As an aside question: should we be initializing the libmamba context ourselves? I was surprised to see that I still needed to set the context bool for the signal handlers, so I don't know if it's still being used behind the scenes for something else.

In C++ you have to construct yourself the context (with whatever options you want) and pass it to other functions afterwards. In python, libmambapy will create itself the context and provide it transparently, and this is mandatory as long as we want to keep the interface as before for python. If we decide we want to break it for v2.0 then you will have to change code using libmamba to create a context yourself and pass it to some other functions. While it does make more sense that way, it's also breaking a lot of interface.

Klaim avatar May 15 '24 14:05 Klaim

This pr should make sure that the test script will work with KeyboardInterrupt (it works at least for me on Windows) It will need some testing.

Other than that we agreed with others at Quantstack to go with just breaking the interface and see after if we need to provide help or an api layer for helping with current users integration. The first step is to just make available in python the creation of the Context, then make it mandatory argument for functions that does need it in C++. This will taker time because the python tests needs to be updated accordingly. Then we'll see if we want to make transparent the context argument (by defaulting it in some ways).

Klaim avatar May 16 '24 15:05 Klaim