pressing key with multiple pressed keys

[mbattista]

First try, still crashes bit seems to work as expected otherwise.

[m1k1o]

Deleting is doing inside the same operation, that searching. That might be joined in one operation (e.g. pop) when you find it, you delete it and return it.

[mbattista]

hmm, it now works.

I removed the list because it was only used to keep track of pressed keys, which seemed unnecessary?

perhaps the XkbKeysymToKeycode function is really expensive and that is why it was put in an hashmap on TigerVNC/tigervnc@ac73038?

Every key i tested worked as expected, but there is still one bug, which I cannot pinpoint yet. By hitting shift + key (any) in rapid succession the key is not getting released. It is easy to reproduce and probably should not be merged with this bug.

[mbattista]

Bug input

neko_1  | 2021-04-11 00:09:54,128 DEBG 'neko' stdout output:
neko_1  | DOWN-Before: 65505
neko_1  | 2021-04-11 00:09:54,129 DEBG 'neko' stdout output:
neko_1  | DOWN-After: %!i(xorg._Ctype_int=50)
neko_1  | 2021-04-11 00:09:54,161 DEBG 'neko' stdout output:
neko_1  | DOWN-Before: 65
neko_1  | 2021-04-11 00:09:54,161 DEBG 'neko' stdout output:
neko_1  | DOWN-After: %!i(xorg._Ctype_int=38)
neko_1  | 2021-04-11 00:09:54,264 DEBG 'neko' stdout output:
neko_1  | UP-Before: 65505
neko_1  | 2021-04-11 00:09:54,264 DEBG 'neko' stdout output:
neko_1  | UP-After: %!i(xorg._Ctype_int=50)
neko_1  | 2021-04-11 00:09:54,282 DEBG 'neko' stdout output:
neko_1  | UP-Before: 65
neko_1  | 2021-04-11 00:09:54,282 DEBG 'neko' stdout output:
neko_1  | UP-After: %!i(xorg._Ctype_int=200)

Normal input

neko_1  | 2021-04-11 00:13:46,884 DEBG 'neko' stdout output:
neko_1  | DOWN-Before: 65505
neko_1  | 2021-04-11 00:13:46,884 DEBG 'neko' stdout output:
neko_1  | DOWN-After: %!i(xorg._Ctype_int=50)
neko_1  | 2021-04-11 00:13:47,144 DEBG 'neko' stdout output:
neko_1  | DOWN-Before: 65
neko_1  | 2021-04-11 00:13:47,145 DEBG 'neko' stdout output:
neko_1  | DOWN-After: %!i(xorg._Ctype_int=38)
neko_1  | 2021-04-11 00:13:47,434 DEBG 'neko' stdout output:
neko_1  | UP-Before: 65
neko_1  | 2021-04-11 00:13:47,434 DEBG 'neko' stdout output:
neko_1  | UP-After: %!i(xorg._Ctype_int=38)
neko_1  | 2021-04-11 00:13:47,576 DEBG 'neko' stdout output:
neko_1  | UP-Before: 65505
neko_1  | 2021-04-11 00:13:47,577 DEBG 'neko' stdout output:
neko_1  | UP-After: %!i(xorg._Ctype_int=50)

[mbattista]

OK.
The list was for this case.
Now it works as intended.

Possible memory leak if he keeps releasing the key outside of neko.

[m1k1o]

That memory leak is guarded by go. Every keydown is stored in map. That is periodically checked and keydowns older that 10s are automatically released.

[m1k1o]

Just noticed, CTRL+C and CTRL+V (maybe all of them) are not working?

[mbattista]

Just noticed, CTRL+C and CTRL+V (maybe all of them) are not working?

CTRL-A / CTRL-Z / CTRL-X / CTRL-C / CTRL-V work for me.

I also cannot reproduce the panic.

[m1k1o]

It looks like after a bunch of crashes some keys are hanging unreleased, what causes CTRL+ not working.

I am able to reproduce panic with constant typing while pressing and releasing Shift key.

[mbattista]

I am able to reproduce panic with constant typing while pressing and releasing Shift key.

I was able to reproduce it once.
Switched to inserting the key instead of appending it. Could no longer reproduce it. Can you try?

[m1k1o]

Tried it. Same for me, I can no longer reproduce it. Weird, there must be some data race happening, that never occures when the key is found early.

[mbattista]

My guess would be, since the error is in malloc.c (https://fossies.org/linux/glibc/malloc/malloc.c Line 3964) and hints dirty memory, that there were race conditions which lead to last not getting freed properly.

The whole reason behind the last pointer was, that in the solution from TigerVNC they checked against the last item of the map (if (pressedKeys.find(key) != pressedKeys.end())). So having a pointer which just did that was nice.
But I still do not know why they checked against the map when a key was pressed and not only when a key was released. It seems to work without it, which makes the last pointer is unnecessary. Or there are still bugs to find, which show the reasoning behind this logic.

[m1k1o]

That is true. Now i realized, deleteItem never cared about last variable.

[m1k1o]

I faced some weird bug. While loop was stuck and looping forever, taking 100% of CPU and neko was not usable. There might be problem with thread concurrency. Or similar stuff. Was not able to replicate it again, I'm trying.

[mbattista]

I hoped that the mutex functions of the go function would break up concurrency in the c part.

A quick and dirty fix would be to break all while loops after 120 rounds?
Still the only possible of never ending loops would be when the next pointer links to itself?
Or do you see any other possibility?

[m1k1o]

I thought about it, giving it some maximum would be at least some insurance. But as you said, go should guarantee thread safety by adding mutexes.

I am doing another tests, and want to see if this bug was only caused somehow by my previous tests or not.

[mbattista]

I thought about it, giving it some maximum would be at least some insurance. But as you said, go should guarantee thread safety by adding mutexes.

There are already mutexes in the go part. https://github.com/m1k1o/neko/blob/dev/server/internal/xorg/xorg.go#L96

[m1k1o]

I am not able to reproduce it at all. It must have been some weird edge case after bunch of tests. I am not even sure, if that caused infitine loop or simply xorg crashed in a bad way comsuming 100% of my cpu with some proccess.

I realized, that function you took from tigervnc PR has been changed meanwhile since 2017 in master. We should probably take the latest version: https://github.com/TigerVNC/tigervnc/blob/0946e298075f8f7b6d63e552297a787c5f84d27c/unix/x0vncserver/XDesktop.cxx#L343-L379

It might adress some hidden issues we would face later.

[m1k1o]

Thanks for your changes, that looks good. But I think that code is perfectly fine, and print that should never be reached. It must have been error on my side, problem with my setup or some different bug when trying that. Otherwise I can't explain that.

[m1k1o]

I just changed mostly variable / function names. Since they are not contiained in one C file, but apparetnly are shared throughout all linked C files to neko, wanted to make them explicit. Also I joined xearching & deleting to single function.