kanaka/noVNC — pull request #193 by takaha (2012-09-26 11:33:11+00:00)
Title: Memory leak that repeated key events lead to

There are some problems related to key events.

1. keyDownList is getting huge, which means there is a memory leak.
This happens when you hold a key pressed, which fires keydown and
keypress events repeatedly.

When you press an 'a' key, the following events will be generated.

keydown keyCode=65 (A) which=65 (A) charCode=0
keypress keyCode=0 which=97 (a) charCode=97 (a)
keydown keyCode=65 (A) which=65 (A) charCode=0
keypress keyCode=0 which=97 (a) charCode=97 (a)
keydown keyCode=65 (A) which=65 (A) charCode=0
keypress keyCode=0 which=97 (a) charCode=97 (a)
keydown keyCode=65 (A) which=65 (A) charCode=0
keypress keyCode=0 which=97 (a) charCode=97 (a)
keyup keyCode=65 (A) which=65 (A) charCode=0

There are several keydown and keypress events and one keyup event.
The current algorithm will register all the keydown events on keyDownList
and delete the last one when the keyup event is issued.

I realized this since I usually hold CTRL-h, CTRL-f, CTRL-b, CTRL-p,
CTRL-n, Backspace, Arrows and Space keys down to repeat the same action.

2. Some keystrokes may not fire a keyup event associated with one of the
keydown events. There are some examples:

FireFox 15 for windows and FireFox 14 and earlier won't generate a keyup
event against the ALT keydown event when you hit an ALTGR key. This
behavior seems to be intentional.

keydown keyCode=17 which=17 charCode=0
keydown keyCode=18 which=18 charCode=0
keyup keyCode=17 which=17 charCode=0

Opera 12 won't generate a keyup event against the CTRL keydown event when
you hit an ALTGR key.

keydown keyCode=17 which=17 charCode=undefined
keydown keyCode=18 which=18 charCode=undefined
keyup keyCode=18 which=18 charCode=undefined

Opera 12 won't generate a keyup event against the SHIFT keydown event when
you press SHIFT, press ALT, release SHIFT and release ALT. Is this a bug?

keydown keyCode=16 which=16 charCode=undefined
keydown keyCode=18 which=18 charCode=undefined
keyup keyCode=18 which=18 charCode=undefined

3. It isn't always correct to overwrite the attribute of the previous
keydown event on keyDownList from the keypress handler. There will be
a chance to lose the keyup events against some keydown events.

For example, IE9 won't generate a keyup event against the '#' (keyCode 35)
event when you when you press SHIFT, press '3', release SHIFT and
release '3' a little bit later. Event 'keypress keyCode=35' overwrites
the attribute of event 'keypress keyCode=51' and event 'keypress
keyCode=51' overwrites it again to lose the Event 'keypress keyCode=35.'

keydown keyCode=16 which=undefined charCode=undefined
keydown keyCode=51 (3) which=undefined charCode=undefined
keypress keyCode=35 (#) which=undefined charCode=undefined
keyup keyCode=16 which=undefined charCode=undefined
keydown keyCode=51 (3) which=undefined charCode=undefined
keypress keyCode=51 (3) which=undefined charCode=undefined
keyup keyCode=51 (3) which=undefined charCode=undefined

4. When you hold a certain key pressed, both the client side OS and the
server side OS generate key events repeatedly at the same time against
the same key.

How to fix these:

Use an associative array for keyDownList. It is enough to record one key
event for each key because even if there are several keydown events issued
against the key, there will be only one keyup event generated for this key.

When it is a keypress event whose keycode is the one to be sent to the VNC
server, send a fake keyup event right after it. This leads, you can avoid
letting the server generate key events repeatedly even when the key is
hold pressed. The client is the one generating repeated key events for the key.
And this also solves Problem 3 that the keypress handler doesn't need to
record the info of keypress events for the keyup handler.

With this approach, Problem 2 is partially fixed that it won't leak memory
anymore but it still leaves some keydown events on keyDownList, whose keyup
events won't be generated forever. I will also post the patch to fix it later.