Commit 0bb40ed
Release Manager
gh-40556: Avoid mutating value field of constructed Integer
## Problem
Run the following code
```
a = [Integer(i) for i in range(100)]
try:
b = pow(2^100000, 3^100000, 5^100000)
finally:
del a
```
it will take a while. Press Ctrl-C before it finishes. Then **it will
hang** for a while before printing `KeyboardInterrupt`.
Meanwhile, the similar-looking code
```
a = [int(i) for i in range(100)]
try:
b = pow(2^100000, 3^100000, 5^100000)
finally:
del a
```
does not suffer from the problem.
## Reason
There are several places in SageMath code that does something like the
following
```
cdef Integer x = <Integer>PY_NEW(Integer)
sig_on()
mpz_powm(x.value, ...) # mutate x.value, slow
sig_off()
return x
```
If the user press ctrl-C, the destructor for `x` will be called, but
because `mpz_powm` is interrupted, `mpz_powm` may be in an
**inconsistent state**, so the destructor might do the wrong thing.
## The band-aid fix
The band-aid is described in
#24986 : a function
`sig_occurred()` is implemented in cysignals that checks whether a
signal thrown by `sig_on()` is currently being processed, and the
destructor of `Integer` checks:
```
if sig_occurred():
leak this Integer object just in case
else:
put this Integer object into a common object pool
```
## The issue
There are two problems with this.
* This leaks integers **indiscriminately**. In particular, in the
example above, **all** the 100 `Integer` objects in `a` are leaked, even
though only one is being mutated, being a local variable in the powering
method.
At any point in the code, we know exactly which object is currently
being mutated, as such this indiscriminate leak is unnecessary.
* `sig_occurred()` is **slow**: in the example above, it runs the
**garbage collector** each time it's called. So, the reason for the hang
is that after the user presses Ctrl-C, Python needs to run the garbage
collector 100 times before reporting `KeyboardInterrupt`.
## The actual fix
I look at the example reported in
#24986 — `coerce_actions.pyx`.
The problematic doctest is
```
E = EllipticCurve(GF(5), [4,0])
P = E([2,1,1])
from sage.doctest.util import ensure_interruptible_after
with ensure_interruptible_after(0.001): 2^(10^8) * P
```
by some analysis, I determine that the responsible function is
`_pow_long`.
As such, this pull request makes sure to not call the destructor when
the user interrupts.
After this pull request, we can remove the `sig_occurred()` workaround
introduced in #24986 .
## Other reported issues
* #39224
*
sagemath/cysignals#215 (comment)
* sagemath/cysignals#227
### 📝 Checklist
<!-- Put an `x` in all the boxes that apply. -->
- [x] The title is concise and informative.
- [x] The description explains in detail what this PR is about.
- [x] I have linked a relevant issue or discussion.
- [ ] I have created tests covering the changes.
- [ ] I have updated the documentation and checked the documentation
preview.
### ⌛ Dependencies
<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - #12345: short description why this is a dependency -->
<!-- - #34567: ... -->
URL: #40556
Reported by: user202729
Reviewer(s): Michael Orlitzky, Travis Scrimshaw, user202729
1 file changed
+61
-35
lines changed| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
401 | 401 | | |
402 | 402 | | |
403 | 403 | | |
| 404 | + | |
| 405 | + | |
| 406 | + | |
| 407 | + | |
| 408 | + | |
| 409 | + | |
| 410 | + | |
| 411 | + | |
| 412 | + | |
| 413 | + | |
| 414 | + | |
| 415 | + | |
| 416 | + | |
| 417 | + | |
| 418 | + | |
| 419 | + | |
| 420 | + | |
| 421 | + | |
| 422 | + | |
| 423 | + | |
| 424 | + | |
| 425 | + | |
| 426 | + | |
| 427 | + | |
| 428 | + | |
| 429 | + | |
| 430 | + | |
| 431 | + | |
| 432 | + | |
| 433 | + | |
| 434 | + | |
| 435 | + | |
| 436 | + | |
| 437 | + | |
| 438 | + | |
| 439 | + | |
404 | 440 | | |
405 | 441 | | |
406 | 442 | | |
| |||
447 | 483 | | |
448 | 484 | | |
449 | 485 | | |
| 486 | + | |
| 487 | + | |
450 | 488 | | |
451 | 489 | | |
452 | 490 | | |
| |||
1746 | 1784 | | |
1747 | 1785 | | |
1748 | 1786 | | |
1749 | | - | |
1750 | | - | |
1751 | | - | |
| 1787 | + | |
1752 | 1788 | | |
1753 | 1789 | | |
1754 | 1790 | | |
| |||
1780 | 1816 | | |
1781 | 1817 | | |
1782 | 1818 | | |
1783 | | - | |
| 1819 | + | |
1784 | 1820 | | |
1785 | 1821 | | |
1786 | 1822 | | |
| |||
1863 | 1899 | | |
1864 | 1900 | | |
1865 | 1901 | | |
1866 | | - | |
| 1902 | + | |
1867 | 1903 | | |
1868 | 1904 | | |
1869 | 1905 | | |
| |||
1975 | 2011 | | |
1976 | 2012 | | |
1977 | 2013 | | |
1978 | | - | |
| 2014 | + | |
1979 | 2015 | | |
1980 | 2016 | | |
1981 | 2017 | | |
| |||
2038 | 2074 | | |
2039 | 2075 | | |
2040 | 2076 | | |
2041 | | - | |
| 2077 | + | |
2042 | 2078 | | |
2043 | 2079 | | |
2044 | 2080 | | |
2045 | 2081 | | |
2046 | 2082 | | |
2047 | 2083 | | |
2048 | | - | |
| 2084 | + | |
2049 | 2085 | | |
2050 | 2086 | | |
2051 | 2087 | | |
| |||
2067 | 2103 | | |
2068 | 2104 | | |
2069 | 2105 | | |
2070 | | - | |
| 2106 | + | |
2071 | 2107 | | |
2072 | 2108 | | |
2073 | 2109 | | |
| |||
2299 | 2335 | | |
2300 | 2336 | | |
2301 | 2337 | | |
2302 | | - | |
2303 | 2338 | | |
| 2339 | + | |
2304 | 2340 | | |
2305 | | - | |
| 2341 | + | |
2306 | 2342 | | |
2307 | | - | |
| 2343 | + | |
2308 | 2344 | | |
2309 | | - | |
| 2345 | + | |
2310 | 2346 | | |
2311 | 2347 | | |
2312 | 2348 | | |
2313 | | - | |
| 2349 | + | |
2314 | 2350 | | |
2315 | 2351 | | |
2316 | 2352 | | |
| |||
3584 | 3620 | | |
3585 | 3621 | | |
3586 | 3622 | | |
3587 | | - | |
| 3623 | + | |
3588 | 3624 | | |
3589 | 3625 | | |
3590 | 3626 | | |
| |||
6888 | 6924 | | |
6889 | 6925 | | |
6890 | 6926 | | |
6891 | | - | |
6892 | | - | |
| 6927 | + | |
6893 | 6928 | | |
6894 | 6929 | | |
6895 | 6930 | | |
| |||
7509 | 7544 | | |
7510 | 7545 | | |
7511 | 7546 | | |
7512 | | - | |
7513 | | - | |
7514 | 7547 | | |
7515 | 7548 | | |
7516 | 7549 | | |
| |||
7523 | 7556 | | |
7524 | 7557 | | |
7525 | 7558 | | |
7526 | | - | |
7527 | | - | |
| 7559 | + | |
| 7560 | + | |
7528 | 7561 | | |
7529 | | - | |
| 7562 | + | |
7530 | 7563 | | |
7531 | 7564 | | |
7532 | 7565 | | |
| |||
7556 | 7589 | | |
7557 | 7590 | | |
7558 | 7591 | | |
7559 | | - | |
7560 | 7592 | | |
7561 | 7593 | | |
7562 | 7594 | | |
| |||
7589 | 7621 | | |
7590 | 7622 | | |
7591 | 7623 | | |
7592 | | - | |
7593 | | - | |
7594 | | - | |
7595 | | - | |
7596 | | - | |
7597 | | - | |
| 7624 | + | |
| 7625 | + | |
| 7626 | + | |
| 7627 | + | |
7598 | 7628 | | |
7599 | 7629 | | |
7600 | 7630 | | |
| |||
7606 | 7636 | | |
7607 | 7637 | | |
7608 | 7638 | | |
7609 | | - | |
7610 | | - | |
7611 | | - | |
7612 | | - | |
7613 | 7639 | | |
7614 | 7640 | | |
7615 | 7641 | | |
| |||
7622 | 7648 | | |
7623 | 7649 | | |
7624 | 7650 | | |
7625 | | - | |
| 7651 | + | |
7626 | 7652 | | |
7627 | 7653 | | |
7628 | 7654 | | |
| |||
7659 | 7685 | | |
7660 | 7686 | | |
7661 | 7687 | | |
7662 | | - | |
| 7688 | + | |
7663 | 7689 | | |
7664 | 7690 | | |
7665 | 7691 | | |
| |||
0 commit comments