|
|
| bugs in key buffering, 4.2.1 and 4.3.0 |
|
orz
Member #565
August 2000
|
These are theoretical bugs - they depend upon race conditions or unusual circumstances that are rare and difficult to reproduce. I have not managed to actually observe them in practice. So, it could all be in my head. But I don't think so : ) Bug #1: Re-entry to keyboard handling code, particularly add_key() On all platforms, in both Allegro 4.2.1 and 4.3.0, add_key() can be called even when it is already executing. This can happen in the following ways: In both Allegro 4.2.1 and 4.3.0 add_key attempts to make itself semi-safe for reentry via this naive locking mechanism: buffer->lock++; if (buffer->lock != 1) { buffer->lock--; return; } ... buffer->lock--; which doesn't work reliably, and makes the consequences of reentry potentially much worse. Even when it does work, it deliberately drops a keypress. In Allegro 4.2.1 there is an additional mechanism used: if ((waiting_for_input) && (keyboard_driver) && (keyboard_driver->stop_waiting_for_input)) keyboard_driver->stop_waiting_for_input(); But this mechanism is insufficient (and doesn't seem to do anything useful). Problems this can cause: Both of those problems can't happen when the code is called from within a signal handler or interrupt (including DOS, MacOS9, and, under most circumstances, Linux), because the relative timings of reads and writes for the problems to occur cannot happen since one call pauses the other until it completes. So the only problem on DOS or MacOS9 would be a dropped keypress, which the code is designed to produce occasionally anyway. . This one is somewhat counter-intuitive, but here's how it works: Memory Ordering in Modern Microprocessors, Part I and Part II This briefly mentions various memory consistency models used by various CPUs: Intel discusses their memory consistency model in section 7.2 of this document: edit: Problems this can cause: Fixes for these bugs: add_key() should lock to prevent reentry into it. Better yet, the locking could be done by the callers to it - simulate_keypress() and _handle_key(), which would also prevent reentry into user callbacks. The possibility of reentry into user callbacks is not mentioned in the docs (indeed, the docs imply otherwise, stating that the callbacks occur "in an interrupt context"). Casual testing for reentry will (incorrectly) suggest that it doesn't happen, since the circumstances that trigger it are rare and difficult to reproduce. Either something equivalent to linuxes smp_wmb() should be added to add_key() or locking should be added to ureadkey(). |
|
Elias
Member #358
May 2000
|
In 4.3, key presses are delivered via the events system, which always does proper locking (see src/event.c). So no problems should exist there I hope. Still, for 4.2 (and the 4.x compatibility layer in the 4.3 code which you probably were looking at) you are probably right. At the time this code was written, there was no SMP - not even threads for that matter Still, easy fix is, simply add a lock, after all what for do we have a complete locking API in 4.2. -- |
|
orz
Member #565
August 2000
|
Quote: In 4.3, key presses are delivered via the events system, which always does proper locking (see src/event.c). So no problems should exist there I hope. Still, for 4.2 (and the 4.x compatibility layer in the 4.3 code which you probably were looking at) you are probably right. Yeah, I was referring to the compatibility layer. The purely new stuff looks correct. Quote:
At the time this code was written, there was no SMP - not even threads for that matter Yipes! Quote: Still, easy fix is, simply add a lock, after all what for do we have a complete locking API in 4.2. Yes... just realize it requires locks for both add_key and ureadkey (unless a memory barrier is added to add_key), plus extra if you want user callbacks to not be reentrant. |
|
|