Open timway opened 1 year ago
I've been encountering this on main
too after updating (the test_curses
tests in our suite seem to trigger it). One really annoying side-effect is that it breaks your terminal after the crash:
% ./python.exe -m test --multiprocess 1 --use curses --verbose test_curses
0:00:00 load avg: 1.27 [1/1/1] test_curses process crashed (Exit code -11)
test_has_extended_color_support (test.test_curses.MiscTests.test_has_extended_color_support) ... ok
test_ncurses_version (test.test_curses.MiscTests.test_ncurses_version) ... ncurses_version = curses.ncurses_version(major=6, minor=0, patch=20150808)
ok
test_update_lines_cols (test.test_curses.MiscTests.test_update_lines_cols) ... ok
test_alt (test.test_curses.TestAscii.test_alt) ... ok
test_ascii (test.test_curses.TestAscii.test_ascii) ... ok
test_controlnames (test.test_curses.TestAscii.test_controlnames) ... ok
test_ctrl (test.test_curses.TestAscii.test_ctrl) ... ok
test_ctypes (test.test_curses.TestAscii.test_ctypes) ... ok
test_unctrl (test.test_curses.TestAscii.test_unctrl) ... ok
TERM=xterm-256color
test_attributes (test.test_curses.TestCurses.test_attributes) ... ok
test_background (test.test_curses.TestCurses.test_background) ... ok
test_beep (test.test_curses.TestCurses.test_beep) ... ok
test_borders_and_lines (test.test_curses.TestCurses.test_borders_and_lines) ... ok
test_chgat (test.test_curses.TestCurses.test_chgat) ... ok
test_clear (test.test_curses.TestCurses.test_clear) ... ok
test_color_attrs (test.test_curses.TestCurses.test_color_attrs) ... ok
test_color_content (test.test_curses.TestCurses.test_color_content) ... ok
test_create_windows (test.test_curses.TestCurses.test_create_windows) ...
== Tests result: FAILURE ==
1 test failed:
test_curses
Total duration: 198 ms
Total tests: run=0
Total test files: run=1/1 failed=1
Result: FAILURE
%
I'm not a curses
expert, but I'm not sure that this is something we can fix on our end, so this should probably be closed. It might be worth disabling the crashy tests if we can detect the broken curses
version, though?
CC @Yhg1s as a curses
expert.
The affected tests appear to be test_create_windows
, test_move_cursor
, test_output_character
, test_refresh
, and test_refresh_control
.
FWIW this problem should not arise when using a python from a macOS python.org installer or MacPorts as they supply their own version of ncurses and do not use the system version.
FWIW this problem should not arise when using a python from a macOS python.org installer or MacPorts as they supply their own version of ncurses and do not use the system version.
I can confirm this. I was only able to reproduce it for @timway when using the system version of python.
Downgrading from Xcode 15.0 (curses.ncurses_version(major=6, minor=0, patch=20150808)
) to 14.3 (curses.ncurses_version(major=5, minor=7, patch=20081102)
) fixes it for me.
It sounds like they shipped a potentially broken version of ncurses embedded within xcode 15?
We've been building and linking with ncurses >= 6 in the rest of the world since 2015 without this problem...
It is really strange behavior on Apple's part to "upgrade" to providing 6.0 from 2015 in their 2023 toolchain when ncurses 6.4 came out last year.
We're at the point where it'd be fine for CPython to simply drop the ability to build and link against 6.0 in 3.13+ given that no supported Linux distro platform will be using it anymore by the time we release 3.13. (RHEL 8 shipped with ncurses 6.1 making that a reasonable minimum) -- _I do not expect or encourage anyone to go through and do that to the _curses
module code._
Recommendation: do what we do with python.org builds, never build against apple's ncurses.
If a workaround for this isn't obvious to someone with a macos debugger, it is reasonable to have a configure check blocklist the xcode 15 supplied ncurses and refuse to use it. Spending our time dealing with brazenly outdated libraries embedded in someone elses toolchain that we don't ship our own builds with isn't worthwhile (it'd never end).
Thank you all for looking into this - as next steps I've found the Feedback Assistant and the general feedback form on Apple's website. I'll work to post to those methods and provide any feedback here.
I could also look at writing a small C example and see if I can confirm if the problem is in their ncurses implementation and fully removed from Python or not.
If you want to require a specific version or recommend libedit or ncurses on macOS, you can update the doc: https://docs.python.org/dev/using/configure.html
Issues like this should also be reported to Apple as this is a crash with their copy of Python.
I've filed FB13196764 about this.
Thanks Ronald!
FWIW this problem should not arise when using a python from a macOS python.org installer or MacPorts as they supply their own version of ncurses and do not use the system version.
Our copy of ncurses is ancient though and should be updated to a more recent version. But that's something for a different issue, and needs careful testing because we rely on the system terminfo database.
The root issue is that is_pad
is weakly linked and is only available in macOS 14 (and iOS 17 etc etc), but it's exposed in recent SDKs. This confuses the configure check for HAVE_CURSES_IS_PAD
which will see the function, although it may be unavailable at runtime.
The compiler warns about this with a warning: 'is_pad' is only available on macOS 14.0 or newer [-Wunguarded-availability-new]
.
There is a weird interaction of HAVE_CURSES_IS_PAD, WINDOW_HAS_FLAGS, and NCURSES_OPAQUE because of bpo-25720. My head hurts following all the cases. But I'm pretty sure that there is no need to do a runtime check, because if you are in one of the cases that would need it (Apple, recent SDK) you also have at the very least ncurses 5.7, so it should always be safe to use the non-opaque fallback.
Ugh, just realized the implication that NCURSES_VERSION_MAJOR/MINOR/PATCH don't correspond to the actual libncurses version.
Maybe the magic macOS macro __builtin_available(macOS ...)
should be used for ncurses.
Maybe the magic macOS macro
__builtin_available(macOS ...)
should be used for ncurses.
It shouldn't be necessary. It's a funny decision on Apple's side to use availability attributes on a function like is_pad
. How do you even implement a fallback if the function is not available at runtime?
For ncurses, it should be enough to default to NCURSES_OPAQUE=0
which:
is_pad
(20090906): also exposes is_pad
as a macro, so we never touch the function, and we don't have to worry about runtime checkswin->_flags
inspectable so the current fallback still works(Hope I got this right. It's a bit of a mess to track so I needed to write this down)
If NCURSES_OPAQUE=0
is ever removed from ncurses, and a future Python version will support that release, it will hopefully be far enough in the future that nobody will be concerned about running that Python on macOS < 14 (hello person from the far future who found this bug, please don't be angry at me).
That said, I don't understand why https://github.com/python/cpython/pull/4164 decided to opt out of setting NCURSES_OPAQUE=0
in case the function is available. I see little reason to switch working code to use the opaque functions (unless it needs to use ncurses from parallel threads).
This is what I had in mind https://github.com/python/cpython/compare/main...sorcio:cpython:curses-mac-fix
As a fan of Python limited C API, I would prefer that Python does not use NCURSES_OPAQUE=0. While it works today, it may break tomorrow.
Another way should be found to fix the Python module.
"is_pad: returns TRUE if the window is a pad i.e., created by newpad"
If the function is not available, can we just not make it available in Python?
@vstinner is_pad is not exposed as a function in the Python module. It's used internally in just a few methods (e.g. window.refresh() or window.subwin()) to transparently work on either windows or pads.
Historically no [n]curses implementations exposed it as a function until ncurses 5.7.20090906. When building against an older version of the headers (such as the one included in Apple SDKs prior to Xcode 15), _cursesmodule.c would use the alternative technique that has been supported for 25 or so years: checking for win->_flags & _ISPAD
. Note that with my change _cursesmodule.c would use the is_pad macro if [n]curses.h defines it, rather than relying on internals.
While it works today, it may break tomorrow.
Definitely. If ncurses ever removes the non-opaque option, it would break setups where CPython links system libncurses dynamically and an older library is provided (such as the 5.7 one on macOS < 14). So, yeah, some hypothetical future CPython built in some specific way depending on some hypothetical future ncurses would probably not be able to import system ncurses on macOS 13.6. But I don't think that's an issue!
As a fan of Python limited C API
Uh, does the limited C API make promises about curses? Technically py_curses.h is not an "internal" header, and a user might import it, and defining NCURSES_OPAQUE might get in their way. But this is completely undocumented use, and it's already platform-dependent.
On 11 Oct 2023, at 17:21, Davide Rizzo @.***> wrote:
Maybe the magic macOS macro __builtin_available(macOS ...) should be used for ncurses.
It shouldn't be necessary. It's a funny decision on Apple's side to use availability attributes on a function like is_pad. How do you even implement a fallback if the function is not available at runtime?
For ncurses, it should be enough to default to NCURSES_OPAQUE=0 which:
Without having looked at the source code: I’d prefer using builtin_available instead of disabling is_pad.
I don't understand what "instead of disabling is_pad" means.
Sorry about the slow response, I was traveling at the time.
I don't understand what "instead of disabling is_pad" means.
You wrote:
The root issue is that is_pad is weakly linked and is only available in macOS 14 (and iOS 17 etc etc), but it's exposed in recent SDKs. This confuses the configure check for HAVE_CURSES_IS_PAD which will see the function, although it may be unavailable at runtime.
The compiler warns about this with a warning: 'is_pad' is only available on macOS 14.0 or newer [-Wunguarded-availability-new].
The IMHO correct way to deal with this is to use is_pad
(and other curses functions new in 14.0) only when running on 14.0 or later by using __builtin_available
. This does complicate the code in Modules/_cursesmodule.c
a little, but the amount of change likely can be limited by slightly restructuring the code (such as making py_is_pad
a static inline function instead of a macro). We already do this in a number of other modules, including some much more involved changes in posixmodule.c.
Thanks for the clarification @ronaldoussoren. Hope the sprints went great!
The issue with using __builtin_available
in this specific case is that, at runtime, you don't have a fallback available. Let's say you have something like:
#define HAVE_CURSES_IS_PAD_RUNTIME __builtin_available(macOS 14.0, *)
// ...
if (HAVE_CURSES_IS_PAD_RUNTIME) {
return is_pad(w);
} else {
// ... what goes here? ...
}
If you have NCURSES_OPAQUE=1 (the default in newer ncurses.h), the true branch can use the is_pad()
function, but the false branch doesn't have a way to check if the window is a pad. The check is not optional (i.e. you can't always return false), it's necessary in some code paths for correctness.
You may set NCURSES_OPAQUE=0, so the false branch can work (either by using the is_pad
macro exported by ncurses.h, or by inspecting w->_flags
). But in this case the availability check is unnecessary, because ncurses.h doesn't even expose the is_pad
function symbol (only the macro), and we have nothing to check, because it's always available.
That's why in https://github.com/python/cpython/compare/main...sorcio:cpython:curses-mac-fix I defaulted to NCURSES_OPAQUE=0 and haven't used the runtime availability check.
An alternative might be to have a separate translation unit, let's say a curses_compat.c, which is (conditionally?) compiled with NCURSES_OPAQUE=0, and leave everything else to the ncurses default. The compatibility layer would expose py_is_pad. It would maybe be cleaner, but I didn't want to go this way because it felt overkill for just a single function.
Since I spent a minute researching this, I'm happy to open a PR.
if (HAVE_CURSES_IS_PAD_RUNTIME) {
return is_pad(w);
} else {
// ... what goes here? ...
}
Don't define is_pad
macro if the function is not available. Modules/_cursesmodule.c
can already be built without is_pad() function. Example in _curses_window_echochar_impl():
#ifdef py_is_pad
if (py_is_pad(self->win)) {
return PyCursesCheckERR(pechochar(self->win, ch_ | (attr_t)attr),
"echochar");
}
else
#endif
return PyCursesCheckERR(wechochar(self->win, ch_ | (attr_t)attr),
"echochar");
Or well, just always return 0?
Modules/_cursesmodule.c
can already be built without is_pad() function
Eh, kinda, but not really in a way that solves the problem. It falls back in two different ways, depending on checks done at configure time:
1) if is_pad is a function, define py_is_pad to use that;
2) otherwise: if is_pad is not available[^not-available], but window flags are inspectable, it defines py_is_pad to check flags. This is what happens on older Apple SDKs (which work!) and on any system with ncurses < 5.8.
3) otherwise: don't define py_is_pad
at all, and ignore the distinction between windows and pads. This is meant to support some other curses, and is never the case with ncurses.
[^not-available]: yeah, in the current setup, with ncurses, either is_pad
is a function (detected by configure, which would set HAVE_CURSES_IS_PAD) or it's not used at all. There is no path that uses the is_pad
macro from ncurses.h, which is legal and documented.
What you are suggesting is 3, but ncurses doesn't like that, e.g. curs_pad(3x):
It is not legal to call wrefresh with a pad as an argument; the
routines prefresh or pnoutrefresh should be called instead.
In the example you pasted, this would end up calling wechochar
on a pad instead of pechochar
, which might not work as expected, or impact performance. Note that this might break in a way that is not covered by test_curses. I don't see tests on pads stuff.
On a personal note, I'm getting confused by this conversation. I suggested a fix and tested it. It took some work to figure out the different cases, and I documented that work in this issue. It's not really complicated, only a bit messy with all the historical cruft. But the comments don't address the proposed solution, and seem to argue that some other unspecified solution should be preferable. I have a bucketful of impostor syndrome to handle, and I'd prefer not to feed it more.
I understand that with Xcode 15.0, the is_pad() function is not available, but win->_flags & _ISPAD
can be tested if the NCURSES_OPAQUE
macro is set to 0. For me, it's surprising to have to tune NCURSES_OPAQUE macro and to inspect a structure member starting with an underscore.
But apparently, this issue affects many macOS issues, it's annoying, and I should stop nitpicking. So just use NCURSES_OPAQUE=0 on macOS, and it should "just works", no?
Does someone want to propose a PR? Nobody proposed a PR so far, no?
I understand that with Xcode 15.0, the is_pad() function is not available, but
win->_flags & _ISPAD
can be tested if theNCURSES_OPAQUE
macro is set to 0. For me, it's surprising to have to tune NCURSES_OPAQUE macro and to inspect a structure member starting with an underscore.
Nope. "Inspecting a structure member starting with an underscore" is what cursesmodule.c does now and has been doing for the last couple decades. If you built on Mac before Xcode 15, you used that.
There is a convenient and documented is_pad
macro, defined in Ncurses public headers for a long while now. Since Apple now includes this version, we can use it.
Does someone want to propose a PR? Nobody proposed a PR so far, no?
I asked for early feedback on a diff a couple times above. I will open a PR in a little bit. Maybe it will reduce the confusion.
Suggestion: let's rename the issue to something that better describes the problem.
Is there a workaround? I want to get back to work on my project.
Is there a workaround? I want to get back to work on my project.
I'm no longer able to replicate the issue with Sonoma 14.2.1 and Command Line Tools for Xcode 15.1.
Alternatively, versions from Python.org shouldn't have the issue.
Edit: It looks like 14.2 got some patches for ncurses behavior (https://support.apple.com/en-us/HT214036). While they don't specifically mention this as an issue it does appear to resolve it for me at least.
Yes, downloading the latest directly from python.org made the problem go away. It would be nice if there were something I could do in code, other than telling customers "you need to install a new version of Python on your system"
Crash report
What happened?
MacOS began pushing out updates to XCode Command Line Tools to install 15.0 recently. Upon updating I began having issues with
curses
. This happens with the Python provided by Apple. I'm not aware of the best way to communicate this issue to Apple, hopefully someone here knows who to ping or is watching.Save the below as
curses-segfault.py
:Run the script in
zsh
in MacOS Terminal via:An easy way to see if you got the update is via the terminal by running:
CPython versions tested on:
3.9
Operating systems tested on:
macOS
Output from running 'python -VV' on the command line:
python3 -VV Python 3.9.6 (default, Aug 11 2023, 19:44:49) [Clang 15.0.0 (clang-1500.0.40.1)]
Linked PRs