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

Bug: Control-C on ipython (on extension code using cysignals) is broken #35712

Closed
tornaria opened this issue Feb 19, 2022 · 3 comments · Fixed by #35730
Closed

Bug: Control-C on ipython (on extension code using cysignals) is broken #35712

tornaria opened this issue Feb 19, 2022 · 3 comments · Fixed by #35730
Labels
bug Something isn't working

Comments

@tornaria
Copy link
Contributor

Correct behaviour (using python):

$ python
Python 3.10.2 (main, Jan 15 2022, 03:11:32) [GCC 10.2.1 20201203] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> from cypari2.pari_instance import Pari
>>> a = Pari()(54853908712446157179434453567343931596301333824416758692208077566613224454501131209)
>>> a.factor() # takes ~ 1/2 hour
^CTraceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "cypari2/gen.pyx", line 4361, in cypari2.gen.Gen.factor
KeyboardInterrupt
>>> 

After running a.factor() I hit control-C (the statement takes ~ half hour to finish). Since this is running extension code (pari C library) python is not catching signals; that's why cypari uses cysignals which is supposed to catch the signal and raise the KeyboardInterrupt.

But when running the same code using ipython nothing happens:

$ ipython
Python 3.10.2 (main, Jan 15 2022, 03:11:32) [GCC 10.2.1 20201203]
Type 'copyright', 'credits' or 'license' for more information
IPython 7.31.0 -- An enhanced Interactive Python. Type '?' for help.

In [1]: from cypari2.pari_instance import Pari

In [2]: a = Pari()(54853908712446157179434453567343931596301333824416758692208077566613224454501131209)

In [3]: a.factor() # takes 1/2 hour
^C^C^C^C^C

This bug affects our sagemath which is using our system packages for ipython, cypari2, cysignals, etc. When using the vendored version of these packages in sagemath, this works ok:

$ ./sage -ipython
Python 3.10.2 (main, Jan 15 2022, 03:11:32) [GCC 10.2.1 20201203]
Type 'copyright', 'credits' or 'license' for more information
IPython 7.29.0 -- An enhanced Interactive Python. Type '?' for help.

In [1]: from cypari2.pari_instance import Pari

In [2]: a = Pari()(54853908712446157179434453567343931596301333824416758692208077566613224454501131209)

In [3]: a.factor()
^C---------------------------------------------------------------------------
KeyboardInterrupt                         Traceback (most recent call last)
<ipython-input-3-af0555404bb2> in <module>
----> 1 a.factor()

cypari2/gen.pyx in cypari2.gen.Gen.factor()

KeyboardInterrupt: 

In [4]: 

It's possible that there's something about our packages that is breaking signal handling.

For the record, the package versions are:

  • ipython: 7.31.0 in void, 7.29.0 in sagemath
  • cypari2: 2.1.2 in void, 2.1.2 in sagemath
  • cysignals: 1.11.2 in void, 1.10.3 in sagemath

Downgrading system ipython to 7.29.0 (which I happen to have in my cache) doesn't fix the issue. I will try to build cysignals-1.10.3 as a void package (since 1.11.2 is the first version to be shipped in void) and see if that changes anything.

@paper42 paper42 added the bug Something isn't working label Feb 21, 2022
@tornaria
Copy link
Contributor Author

The issue is fixed by downgrading python3-prompt_toolkit to 3.0.24_1, with every other package as currently in the repos (including the recent update to ipython 8.0.1).

The issue is present with prompt_toolkit 3.0.26_1 so it seems to be a regression between 3.0.24 and 3.0.26 (I don't have 3.0.25 in my cache).

@tornaria
Copy link
Contributor Author

tornaria commented Feb 21, 2022

There's a strong smell in the changelog for 3.0.25 and 3.0.26:

3.0.26: 2022-01-27
------------------

Fixes:
- Fixes issue introduced in 3.0.25: Don't handle SIGINT on Windows.


3.0.25: 2022-01-27
------------------

Fixes:
- Use `DummyOutput` when `sys.stdout` is `None` and `DummyInput` when
  `sys.stdin` is `None`. This fixes an issue when the code runs on windows,
  using pythonw.exe and still tries to interact with the terminal.
- Correctly reset `Application._is_running` flag in case of exceptions in some
  situations.
- Handle SIGINT (when sent from another process) and allow binding it to a key
  binding. For prompt sessions, the behavior is now identical to pressing
  control-c.
- Increase the event loop `slow_duration_callback` by default to 0.5. This
  prevents printing warnings if rendering takes too long on slow systems.

Edit: built 3.0.25 which is already broken.

@tornaria
Copy link
Contributor Author

Reported upstream: prompt-toolkit/python-prompt-toolkit#1576

tornaria added a commit to tornaria/void-packages that referenced this issue Feb 21, 2022
tornaria added a commit to tornaria/void-packages that referenced this issue Feb 24, 2022
tornaria added a commit to tornaria/void-packages that referenced this issue Feb 28, 2022
jcul pushed a commit to jcul/void-packages that referenced this issue Feb 28, 2022
gmbeard pushed a commit to gmbeard/void-packages that referenced this issue Mar 12, 2022
algor512 pushed a commit to algor512/void-packages that referenced this issue Mar 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants