Message1108

Author jbms
Recipients
Date 2013-02-05.08:34:43
Content
There seem to be several bugs due to the way the input system is using coroutines.

- As a side note, the system seems designed to allow multiple nested in-progress key sequences, which
makes things more complicated and more difficult to understand.  As far as I can tell, this is only
needed to support using prefix commands with M-x.  Maybe the added complexity is nonetheless worth it,
though.

- I haven't looked at the code to fully understand how everything interacts with
window.input.begin_recursion()/end_recursion(), but I believe it may need to be added even for
minibuffer_message_state, as for instance minibuffer.show_wait_message (used by minibuffer.wait_for, in
turn used by some invocations of an external editor, for example) depends on having a different key map
working.  It is also not totally clear if these should be called in load/unload rather than in the
constructor/destroy.

- When running normal commands, input_handle_sequence, which is itself a long-running coroutine, invokes
the commands as part of its own coroutine using "yield call_interactively(...)".  This is a problem
because the command may indeed make use of the coroutine, and take arbitrarily long, waiting for external
prorgams, for things to download, etc.  Meanwhile, when the interactive command is waiting on something
internally to finish, a key event handlers like input_handle_keypress, etc. might be invoked by Mozilla,
and since the continuation is still active, will unexpectedly wake up the coroutine with a key event
object.  For example, some other random function like in spawn-process.js will have done "var x = yield
SUSPEND;" and expect to be woken up by some internal mechanism with some appropriate value or exception.
 Instead, though, it gets x assigned to a key event object that it isn't expecting (and that key event
object never makes it to input_handle_sequence where it belongs).  In short, if a key event happens while
the coroutine is still being used by the command, there will be undefined behavior.  

  A specific case where this breaks something is with edit-current-field-in-external-editor.

  The most direct solution is to instead invoke the command in a new "fire-and-forget" coroutine using
e.g. co_call(call_interactively(...)).

- Prefix commands present a special problem because the above solution doesn't quite work.  The prefix
command is supposed to affect the current key sequence context, so we can't just fire it off
asynchronously and proceed without waiting for it to be done.  However, it isn't clear what should happen
if a key event comes in while we are waiting for a prefix command to finish running.  Probably the answer
would have to be either ignoring it, or queuing it up somehow.  Either way it feels like an unnatural
thing to do.  The situation is significantly simplified if prefix commands are required to be regular,
synchronous functions and cannot be coroutine functions, as the hook functions that they replaced were. 
In that case, we can safely run the prefix command immediately and not have to worry about what to do
while waiting for it to finish.  Indeed, all of the prefix commands in conkeror are regular synchronous
functions, with one exception: execute-extended-command.  In practice, this works because
execute-extended-command calls minibuffer.read_command, which calls window.input.begin_recursion, which
prevents the input system from clobbering the existing continuation being used by
execute-extended-command.  That is also why call_after_timeout is needed later, as otherwise
input_handle_command would attempt the invalid operation of waking up the already-running continuation. 
Of course this does mean there is a small window during which a random other key could be pressed and get
processed before input_handle_command is called, though I don't think that is necessarily a practical
problem.  I think there might be an elegant solution that does not require nested active key sequences,
though I haven't thought through all of the details: maybe having execute-extended-command store the
current input state and then tell the input system to use it once it finishes running the specified command.
History
Date User Action Args
2013-02-05 08:34:43jbmssetmessageid: <1360053283.68.0.781696636321.issue440@bugs.conkeror.org>
2013-02-05 08:34:43jbmslinkissue440 messages
2013-02-05 08:34:43jbmscreate