| msg47091 - (view) |
Author: Michiel de Hoon (mdehoon) * |
Date: 2004-10-19 09:03 |
| PyOS_InputHook is a pointer to a function to be called periodically when Python is idle. It is used, for example, to get messages delivered to graphics windows. If I compile Python from source (which uses Modules/readline.c), PyOS_InputHook is called ten times per second. However, with the Windows-version of Python, PyOS_InputHook is called only once for each command typed by the user. It seems that the problem resides in Parser/myreadline.c (which I presume is used for the Windows-version of Python): if (PyOS_InputHook != NULL) (void)(PyOS_InputHook)(); ... p = fgets(buf, len, fp); Python idles at "fgets", but PyOS_InputHook is not being called while Python is idle. The attached patch solves this problem by using the "select" function, basically in the same way as what is in Module/readline.c. I don't know how to compile Python for Windows, so I wasn't able to test this patch. |
|
|
| msg47092 - (view) |
Author: Michiel de Hoon (mdehoon) * |
Date: 2005-03-26 13:33 |
| Logged In: YES user_id=488897 I have now tested this patch. After fixing one error (I had forgotton to declare one variable), the patch works correctly. I have uploaded a fixed patch. Note that this bug occurs not only on Windows, but on any Python compiled without readline. (which allowed me to test this patch by compiling Python without readline support). |
|
|
| msg47093 - (view) |
Author: Kurt B. Kaiser (kbk) *  |
Date: 2005-04-30 18:21 |
| Logged In: YES user_id=149084 The revised patch looks ok to me. |
|
|
| msg47094 - (view) |
Author: Michiel de Hoon (mdehoon) * |
Date: 2005-05-02 02:52 |
| Logged In: YES user_id=488897 Thanks for looking at this patch. Today I found out that it is possible to compile Python 2.4.1 with the older Microsoft Visual Studio 6.0, which I happen to have in my office. It turned out that the patch does not work correctly on Windows, due to the fact that the "select" function doesn't work with stdin on Windows. I will fix the patch so it'll work on Windows too. |
|
|
| msg47095 - (view) |
Author: Kurt B. Kaiser (kbk) * |
Date: 2005-05-02 03:40 |
| Logged In: YES user_id=149084 Yeah, that's why I didn't want to check it in, I can't build on Windows ithe VC 5. |
|
|
| msg47096 - (view) |
Author: Michiel de Hoon (mdehoon) * |
Date: 2005-05-13 03:57 |
| Logged In: YES user_id=488897 I have rewritten the patch to include Windows support. I compiled Python on Windows with VC98, and on Linux and Mac OS X and found no problems with it. The new patch is attached. |
|
|
| msg47097 - (view) |
Author: Michiel de Hoon (mdehoon) * |
Date: 2005-06-10 20:46 |
| Logged In: YES user_id=488897 Thomas Heller sent me this comment: > The PyOS_StdioReadline function calls my_fgets with a file > pointer argument. The my_fgets function in the patch assumes > that STD_INPUT_HANDLE is the handle to use - is this > assumption always correct? He is right, this assumption is not necessarily correct. I have made a new patch (labeled 20050610) to solve this issue. This latest version has been tested on Cygwin, but not yet on Windows -- I need to dig up a compiler for Windows first. |
|
|
| msg47098 - (view) |
Author: Michiel de Hoon (mdehoon) * |
Date: 2005-07-16 02:14 |
| Logged In: YES user_id=488897 I have now recompiled Python on Windows to test this patch. When compiling this patch, I found out that _get_osfhandle needs to be cast to a HANDLE*. The latest version of this patch (dated 20050715) includes this cast. I have tested this patch by running the test suite. One note: Whereas "python -i inputfile.py" and "python < inputfile.py" work as expected, "python -i < inputfile.py" behaves differently with this patch (python waits for the user to hit enter before proceeding). I am not sure if this is significant, as I wouldn't know what a user might try to achieve with "python -i < inputfile.py". I'd be happy to send a binary for Windows to anybody who would like to test this patch. |
|
|
| msg47099 - (view) |
Author: Michiel de Hoon (mdehoon) * |
Date: 2005-07-29 02:40 |
| Logged In: YES user_id=488897 I have found a solution for the problem described below ("python -i < inputfile.py" behaves differently). The solution was to use _isatty to check if stdin is redirected, and check for keyboard input only if stdin is not redirected. With the latest version of this patch (20050728), there are no changes in Python's behaviour on Windows or Unix (except that PyOS_InputHook is called ten times per second, which is what this patch intends to solve). I apologize for the fact that multiple iterations were needed to converge to the right (I hope) solution for this bug. |
|
|
| msg47100 - (view) |
Author: Michiel de Hoon (mdehoon) * |
Date: 2005-08-05 21:07 |
| Logged In: YES user_id=488897 One more fix: I noticed that on Windows, the patch causes the CPU usage to go up to 100%. The reason for this is that if the users presses a control key (e.g. shift), the WaitForSingleObject function returns, but _kbhit() returns false since no character input is waiting in the buffer. Hence, the loop continues to run, and WaitForSingleObject will again return because the control key stroke has not been flushed from the buffer. The fix is in the patch dated 20050803, which is to flush the buffer with FlushConsoleInputBuffer if a key was hit but no character input is available. |
|
|
| msg47101 - (view) |
Author: Martin v. Löwis (loewis) * |
Date: 2005-08-07 18:25 |
| Logged In: YES user_id=21627 I think this code is way too fragile to be applied to 2.4. As for specific errors I could find: - if the system is not windows, it invokes select unconditionally; this should be conditioned with HAVE_SELECT - the Windows version uses kbhit; this may or may not operate on the same handle as hStdIn. In fact, it likely is a different handle. See the source code of the crt: kbhit uses _coninpfh, which is created from opening CONIN$. Try using GetNumberOfConsoleInputEvents/PeekConsoleInput instead, checking for a KEY_EVENT event. - as stdin may be buffered, it may be the case that there is already input available, even though the file descriptor is not ready. |
|
|
| msg47102 - (view) |
Author: Michiel de Hoon (mdehoon) * |
Date: 2005-09-24 23:54 |
| Logged In: YES user_id=488897 Your comments made me realize that since fixing PyOS_InputHook's behavior is potentially too disruptive, it's probably better not to do that with a simple patch. So I think it's best to reject this patch. PyOS_InputHook and event loops in general probably should be discussed in some more detail on python-dev before making changes to Python. For the record, I wrote a response to the three issues you raised. You are right of course on #1, but I don't think #2 and #3 are valid --see below. Thanks for looking at this patch. #1: > if the system is not windows, it invokes select > unconditionally; this should be conditioned with HAVE_SELECT You are certainly right here. While this patch can probably be fixed, this code would have to be tested on a large number of platformsto make sure this patch won't break python for somebody (which is why I'm shying away from doing this in a patch right now). #2 > the Windows version uses kbhit; this may or may not > operate on the same handle as hStdIn. In fact, it likely is > a different handle. See the source code of the crt: kbhit > uses _coninpfh, which is created from opening CONIN$. Try > using GetNumberOfConsoleInputEvents/PeekConsoleInput > instead, checking for a KEY_EVENT event. In the patch, I test if the file descriptor is a tty. The _kbhit() function is called only if that is the case. So AFAICT, using _coninpfh is the right thing to do. Note also that the same code is already being used in _tkinter: In the EventHook function, _kbhit is used to decide if the program should return from the EventHook function because input is available at stdin. So essentially, I am moving code from _tkinter to Python's main loop, to make it available for extension modules other than _tkinter. I'm not sure how GetNumberOfConsoleInputEvents or PeekConsoleInput would help us. #3 > as stdin may be buffered, it may be the case that there is > already input available, even though the file descriptor is > not ready. I don't think the situation can occur in which select blocks although input is already available in the buffer used by fgets. With thanks to comp.unix.programmer: If python is reading input from a tty, the input mode is line buffered. Each time the user hits return, a complete line is available to fgets, so buffering is not a problem. If, however, python is reading input from disk, the input mode is block buffered. When the end of the file is reached, EOF is flagged, and select will return immediately. So if fgets reads a block of data and stores it in its buffer, and still more data is available in the file, the file descriptor will be ready and select returns. If, however, fgets reads the last block of data and stores it in its buffer, EOF is set, so select won't block in this case either. Note that this is also code that already exists in Python's _tkinter. If _tkinter is imported, before calling fgets, Python sits in a loop inside the EventHook function. Except on Windows platforms, the fileno of stdin is passed to Tcl_CreateFileHandler, where it is used in a call to select inside the Tcl_WaitForEvent function. The call to select will trigger a call to MyFileProc in _tkinter.c if input is available, causing EventHook to return so that fgets can be called. So essentially, the same code exists already inside _tkinter (with the call to select buried inside Tcl). |
|
|
| msg47103 - (view) |
Author: Martin v. Löwis (loewis) * |
Date: 2005-09-25 04:29 |
| Logged In: YES user_id=21627 Rejecting as suggested. |
|
|