GNOME Bugzilla – Bug 765382
Terminal not working anymore with vte 0.44
Last modified: 2016-04-25 03:27:29 UTC
Created attachment 326498 [details] Remmina VTE terminal issue I'm one of the Remmina developers, there is probably something wrong in our code that breaks the VTE plugin since the release of VTE 0.44. Enclosed you can see a screenshot of the issue, then only visible thing is the cursor, but whatever I do nothing happens (I've tried to blindly execute an ls > /tmp/foo.log). It's like the ssh process is not spawned (well... we don't spawn anything at all in reality). I cannot understand where is the issue, I've compared our code with other VTE based terminals, like sakura, and from my (n00b) point of view, it should work out of box. I'm really lost and I'd appreciate your help, at least to point me into the right direction (like how to debug, etc) If you have some time to take a look at our code... This is the code base I'm working on to try to fix the issue. https://github.com/FreeRDP/Remmina/blob/vte3/remmina/src/remmina_ssh_plugin.c This is a plugin that will start an ssh session using VTE as the terminal. In the case you need in our wiki at https://github.com/FreeRDP/Remmina/wiki/Compile-on-Ubuntu-16.04 we have instructions on how to compile it. Thanks in advance!!!
An additional information: It has been reported, that Remmina works if rolling back vte to the version 0.42.5 https://bbs.archlinux.org/viewtopic.php?id=211403
I can't recall any change that could be relevant. (Perhaps commit 659d64b ?) Could you please do a git bisect to locate the single vte commit that breaks it? (I'm not following you wrt. not spawning anything in the terminal, and can't find any reelevant code in the file you linked. How do you start the ssh client, then?)
Did you try strace -f to see if everything is alright with the PTY ?
Is it broken only if Remmina is compiled against vte-0.42, gtk+-3.18 etc. (as found in Xenial) and later these components are upgraded, or is it also broken if Remmina is compiled against vte-0.44, gtk+-3.20 etc.? (Im the spirit of bug 763538 / commit fa008f96)
Thanks a lot for your answers! It's amazing how fast you are :-) I'm not be able to test for some hours as I'm going back at home. @Egmont > git bisect I don't have (yet) a GNOME development environment, my bad... So I cannot really build most of the packages separately without building a monster on my laptop. I'll see how to build a GNOME dev environment ASAP. > Where in the code I spawn the ssh connection I'll explain you better later. > Libs It is broken only if is compiled against gtk+-3.20 and any version above vte-0.42.5. On Gtk+-3-18 with 0.42 it works fine. @Christian Good point! I didn't, I'll do as I'll be back at home (hopefully)
Try building vte with --enable-debug, set the env var VTE_DEBUG=pty and see if that gives any additional error information. Also, I noticed you pass the PTY "slave" FD to vte_pty_new_foreign, but this function expects the master FD.
I can reproduce the problem with Ubuntu's remmina package and gtk 3.18, only changing the vte package. I'm running git bisect now...
I'd expect the problematic commit to be one of the PTY cleanup ones.
Back to business... I'm going to do some tests as well, I'll try to provide you all the info you asked for, I hope I'll be as fast as you are :-)
It was broken by commit 3720b4d27a1d79247db9e368c09572beb42580b1 Author: Egmont Koblinger <egmont@gmail.com> Date: Sun Nov 29 20:57:54 2015 +0100 pty: Use packet mode on the PTY This allows us to get informed when scroll lock changes. https://bugzilla.gnome.org/show_bug.cgi?id=755371 (Why was this committed 3 times and reverted twice? Nevermind...) Following this change, you get the "staircase" effect and the first character of each chunk of data stripped off, that is, tons of display corruptions. A bit later, commit 3816196e56a750888425e1460f1e4077c5a92dfa Author: Christian Persch <chpe@gnome.org> Date: Sun Nov 29 20:57:55 2015 +0100 pty: Also set packet mode on foreign PTY changed the behavior to the entirely blank canvas.
In fact I see that the ssh connection is done > 1859 poll([{fd=8, events=POLLIN}], 1, -1 <unfinished ...> > 1868 recvfrom(9, "SSH-2.0-OpenSSH_6.9\r\n", 4096, 0, NULL, NULL) = 21 > 1868 sendto(9, "SSH-2.0-libssh-0.7.3\n", 21, 0, NULL, 0) = 21 > 1868 poll([{fd=9, events=POLLIN|POLLOUT}], 1, 9993) = 1 ([{fd=9, revents=POLLOUT}]) > 1868 poll([{fd=9, events=POLLIN}], 1, 9993 <unfinished ...> Let me know if I can test anything else. Thanks!
(In reply to Christian Persch from comment #6) > Also, I noticed you pass the PTY "slave" FD to vte_pty_new_foreign, but this > function expects the master FD. Does fixing that also fix this bug here?
So... (I don't get the whole picture and I might ask stupid questions or use wrong terminology) What does Remmina do differently than a manually issued "ssh remotehost" (which works correctly)? Who implements the tty driver in this case? Could it be somewhere else rather than in the kernel (e.g. in Remmina, or in ssh)? Could it be a driver that does not support packet mode? Is there any chance of fixing this properly (if so then where? in vte? in remmina? in ssh?), or should we just revert this entire packet mode stuff? (We never got around to finish the scroll lock stuff anyway.)
It is implemented by libssh as described here [1] and here [2] In the remmina code, the function is called in remmina_ssh.c [3] On Remmina side I'm afraid we can't do anything, except migrating the libssh code to openSSH (I'm tempted but it's quite a big task). I could open a bug to libssh, but could it be possible to temporary revert the two commits till libssh will answer? In the case tell me a bit more please so that I can explain well what is needed. [1]. http://api.libssh.org/master/libssh_tutor_shell.html#opening_shell [2]. http://api.libssh.org/master/group__libssh__channel.html#ga37c1cec33fe5a2f184768aba52e3a0db [3]. https://github.com/FreeRDP/Remmina/blob/vte3/remmina/src/remmina_ssh.c#L1499
(In reply to Antenore Gatta from comment #14) > I could open a bug to libssh, but could it be possible to temporary revert > the two commits till libssh will answer? In the case tell me a bit more > please so that I can explain well what is needed. I'm also just trying to understand what happens and probably don't know anything more than you do :) Before reverting anything, I'd like to have a better understanding of the story. Also, I'm hoping that Christian's idea (comment 12) will fix it.
I don't think packet mode should be reverted; it's on my list for this cycle to finish this (maybe even look at the kernel stuff that's missing). https://github.com/FreeRDP/Remmina/blob/vte3/remmina/src/remmina_ssh.c#L1599 says the PTY is opened just like we do, but then they pass the "slave" FD to vte instead of the master. That needs to be fixed _regardless_, and please try to see if it fixes this bug.
Created attachment 326519 [details] [review] remmina patch to use pty master Not sure if I'm doing it right, but it does not fix the bug for me :(
Review of attachment 326519 [details] [review]: Please forget my previous comment/patch. The patch is incorrect, it even breaks Remmina with vte-0.42.
No, it looks fine but I think you have generated it starting from the "next" branch (our master)... Let me take a look, I'm not too fast sorry :-P
Yeah I used "next", as instructed by the wiki you linked :) Unpatched, with vte 0.42 => works. With my patch, same vte => does not work. So it's something wrong with my patch. We'd need a patch that does not break with vte 0.42 to begin with, and then see if it fixes vte 0.44 :)
I don't think .slave is needed at all in ssh_plugin, so you don't need to keep it. So it seems that they open a SSH channel, read from it and forward what's written to (our) PTY. However, remmina_ssh_shell_thread uses the master FD, shouldn't that be the "slave" FD throughout that function ?
Created attachment 326522 [details] [review] remmina fix Yup, thanks Christian, this works! Both fixes (pass the master to vte_whatever() instead of the slave, and reading/writing from/to the slave instead of the master) are needed at the same time.
Yes! It works on 0.42 ... You guys are too fast for me lol I'll lurk much more often here. I'm going to try with 0.44
BTW... I'm going to simply commit in our branch (with some comments). Afterwards I'll update the AUTHORS files with your names... let me know if you prefer otherwise.
I confirm that IT WORKS also with 0.44!!!! Thanks a lot guys! I'm positively puzzled :-)
Let's close this then :-)
(In reply to Antenore Gatta from comment #24) > Afterwards I'll update the AUTHORS files with your names... let me know if > you prefer otherwise. This is very kind of you, however, as far as I'm concerned, your "thanks" here was more than enough for me :) I was happy to help and I definitely don't consider myself a Remmina developer :) Also, the bug was found and essentially the fix was created by Christian, I just mechanically converted his instructions into an actual patch file, the credits go to him. (I've no clue about Christian's preference.)
Just as a final remark, this came about because you're not doing error checking: vte_terminal_set_pty (terminal, vte_pty_new_foreign_sync(master, NULL, NULL)); Instead, you should create the VtePty separately and check for error return.
Wise words. There is a lot of work to (re)do in Remmina.