bpo-29700: try not to crash when using readline with high FDs.

[marienz]

We used the input FD for readline with select(), which crashes if it is >1024.

Fix it by adding an implementation based on poll().

This does not fix it on all platforms, because poll() does not work correctly on
terminals on some platforms. For now, only use the new implemetation on Linux.

This also does not fix it on all versions of readline, as some of them use
select() internally. Those crash the same way we used to.

Fixes issue 29700.

[the-knights-who-say-ni]

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA).

Unfortunately we couldn't find an account corresponding to your GitHub username on bugs.python.org (b.p.o) to verify you have signed the CLA. This is necessary for legal reasons before we can look at your contribution. Please follow these steps to help rectify the issue:

1. If you don't have an account on b.p.o, please create one
2. Make sure your GitHub username is listed in "Your Details" at b.p.o
3. If you have not already done so, please sign the PSF contributor agreement. The "bugs.python.org username " requested by the form is the "Login name" field under "Your Details".
4. If you just signed the CLA, please wait at least one US business day and then check "Your Details" on bugs.python.org to see if your account has been marked as having signed the CLA (the delay is due to a person having to manually check your signed CLA)
5. Reply here saying you have completed the above steps

Thanks again to your contribution and we look forward to looking at it!

[marienz]

CLA should be good now (why do the-knights-who-say-ni need me to tell them that? I assumed they'd re-check automatically...).

[vadmium]

Maybe __linux__? https://sourceforge.net/p/predef/wiki/OperatingSystems/#linux-kernel

[nascheme]

It is bad practice to add tests like "OS == Linux". That's similar to browser sniffing, e.g. "This page only works in IE6". Instead, you want to check if the platform supports the specific operation you need. In this case, maybe that would require a configure test that looks if poll() can be called on terminal fds (ttys?). It has been some time since I've done configure hacking but if you pester me, I could look into how to do it. Not sure about the test in the case of cross-compiles, I guess the same as many other tests of platform behaviour.

If you do have to test for certain platforms, OSes, etc, it is much better to have a blacklist rather than a whitelist. E.g. if you know poll() is broken on FreeBSD < version X.Y then add a test for that. That's still worse than just testing if the function actually works how you want though.

The structure of the code is pretty messy as well. This is leading down the path of "ifdef hell". I.e. in a few years, no one will understand the maze of ifdefs and which are actually needed anymore. Maybe have two separate functions, one for platforms that support poll() and another for ones that don't? I'd have to look at the patch more closely to figure out how to clean it up but I don't like all the preprocessor tests intermixed within the functions.

[marienz]

My dilemma with whitelisting vs blacklisting is that poll() is only an advantage if this code gets called with a high FD (which is rare), I can't conveniently test on the one OS I've heard is broken (some versions of OS X), and I've since heard rumours that some other BSDs may also be broken, but I don't know exactly which ones or how. I'm trying to avoid (mostly) unnecessarily regressing OSes I can't test on.

Testing at configure time is incorrect if the bug is only present on some versions of the OS (which according to https://bugs.python.org/issue26870#msg265604 it is, only OS X 10.4 and 10.6 are broken), since Python won't necessarily be built on the same OS version it is run on.

Is blacklisting OS X 10.4 and 10.6 (and waiting for bug reports if any other OSes are also broken) ok?

Agreed the ifdefs are getting messy (there's already a fair number of them, but that's not a good excuse). It'd need some restructuring to support poll and select using separate functions without a lot of duplicated code. I can try to find a way of cleaning that up, once the whitelist vs blacklist thing has been addressed.