Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Trouble shutting down terminals #5061

Open
t-makaro opened this issue Aug 6, 2018 · 16 comments · Fixed by #6339
Open

Trouble shutting down terminals #5061

t-makaro opened this issue Aug 6, 2018 · 16 comments · Fixed by #6339

Comments

@t-makaro
Copy link

@t-makaro t-makaro commented Aug 6, 2018

From the launcher, I open a terminal (windows PowerShell). I go to the running tab on the sidebar, and click shutdown. The terminal displays [Finished... Term Session]. The terminal reappears in the running sidebar. At this point a couple of things might happen:

  1. I close the terminal tab, and reopen it from the running sidebar. It no longer displays [Finished... Term Session]. If I try to shut the terminal down again, it will restart. (I'm right back where I started)

  2. If I leave the terminal tab open with [Finished... Term Session] displayed and click shutdown a second time, the terminal shuts down without restarting.

Here is a gif with the server console open on the right: (open in new tab to see it larger)
2018-08-06_11-30-08

Also, if I close the terminal tab while it is still running when I hit shutdown. The terminal tab reopens, which is super annoying. It should just be gone.

Here is a gif with the server console open on the right: You can see an error is thrown.
2018-08-06_11-34-34

@blink1073
Copy link
Member

@blink1073 blink1073 commented Aug 7, 2018

Confirmed on Windows 10 with Chrome 68 and JupyterLab 0.33.7. I did not see the behavior on MacOS with the same Chrome and JupyterLab versions. Thanks for the report!

@blink1073 blink1073 added this to the 1.0 milestone Aug 7, 2018
@liffiton
Copy link

@liffiton liffiton commented Aug 9, 2018

I am seeing similar issues with JupyterLab 0.33.8 running from a Jupyterhub install on Linux. Finished terminals restart themselves under a variety of conditions. The simplest way for me to reproduce it is to open a new terminal and immediately press CTRL+D. The terminal session reports it has terminated, but a second or two later I get a new active prompt:

user@host ~
$ exit

[Finished... Term Session]
user@host ~
$

I also get similar results to those reported above when I use the "Running" side tab to shut down active terminal sessions.

This issue was not present when I was running Jupyterlab 0.32. It started for me as soon as I upgraded to an 0.33.x version.

@gdlmx
Copy link

@gdlmx gdlmx commented Sep 2, 2018

I also observe this issue on Linux , with version 0.34.6. There're two issues:

  1. After quiting the shell inside the terminal and seeing [Finished... Term Session], the terminal restarts automatically. The only way of a complete shutdown is using the "task manager" panel.
  2. If two browser-tabs (windows) connect to the same Jupyterlab server simultaneously and a terminal is launched in one of them, no matter how you shut it down in the other tab (even using the "task manager"), it always manages to resurrect.

After some digging, I find out the reason behind both phenomenon, which gets down to the browser-server interaction. Below are the server's log and the browser's log:

[I 06:07:22.811 LabApp] EOF on FD 21; stopping reading
[I 06:07:22.912 LabApp] Terminal 2 closed
Websocket closed
TermSocket.open: 2
[I 06:07:24.080 LabApp] New terminal with specified name: 2
TermSocket.open: Opened 2

image

The message New terminal with specified name: 2 appears immediately after the browser sends the websocket request 2?token=... to the server. It seems that the server blindly starts up a terminal when receiving the websocket request with a valid token.

Moreover, sometimes the following error occurs when the websocket closes:

[I 06:51:51.622 LabApp] Terminal 3 closed
Websocket closed
[E 06:51:51.662 LabApp] Uncaught exception, closing connection.
	Traceback (most recent call last):
	  File "/lib/python3.6/site-packages/tornado/iostream.py", line 752, in wrapper
		return callback(*args)
	  File "/lib/python3.6/site-packages/tornado/stack_context.py", line 300, in null_wrapper
		return fn(*args, **kwargs)
	  File "/lib/python3.6/site-packages/tornado/websocket.py", line 488, in on_connection_close
		self.on_close()
	  File "/lib/python3.6/site-packages/terminado/websocket.py", line 95, in on_close
		self.terminal.resize_to_smallest()
	  File "/lib/python3.6/site-packages/terminado/management.py", line 61, in resize_to_smallest
		rows, cols = self.ptyproc.getwinsize()
	  File "/lib/python3.6/site-packages/ptyprocess/ptyprocess.py", line 779, in getwinsize
		x = fcntl.ioctl(self.fd, TIOCGWINSZ, s)
	ValueError: file descriptor cannot be a negative integer (-1)
[E 06:51:51.663 LabApp] Exception in callback functools.partial(<function wrap.<locals>.null_wrapper at 0x2b10cab172f0>)
	Traceback (most recent call last):
	  File "/lib/python3.6/site-packages/tornado/ioloop.py", line 758, in _run_callback
		ret = callback()
	  File "/lib/python3.6/site-packages/tornado/stack_context.py", line 300, in null_wrapper
		return fn(*args, **kwargs)
	  File "/lib/python3.6/site-packages/tornado/iostream.py", line 752, in wrapper
		return callback(*args)
	  File "/lib/python3.6/site-packages/tornado/stack_context.py", line 300, in null_wrapper
		return fn(*args, **kwargs)
	  File "/lib/python3.6/site-packages/tornado/websocket.py", line 488, in on_connection_close
		self.on_close()
	  File "/lib/python3.6/site-packages/terminado/websocket.py", line 95, in on_close
		self.terminal.resize_to_smallest()
	  File "/lib/python3.6/site-packages/terminado/management.py", line 61, in resize_to_smallest
		rows, cols = self.ptyproc.getwinsize()
	  File "/lib/python3.6/site-packages/ptyprocess/ptyprocess.py", line 779, in getwinsize
		x = fcntl.ioctl(self.fd, TIOCGWINSZ, s)
	ValueError: file descriptor cannot be a negative integer (-1)
Websocket closed

In human language, that is: This tornado.websocket object contains an invalid self.terminal reference.

@jasongrout jasongrout removed this from the 1.0 milestone Sep 5, 2018
@blink1073 blink1073 added this to the Future milestone Sep 7, 2018
@t-makaro
Copy link
Author

@t-makaro t-makaro commented Sep 12, 2018

@blink1073 I feel like this is a fairly bad bug and should be fixed for 1.0 if possible.

@blink1073
Copy link
Member

@blink1073 blink1073 commented Sep 12, 2018

I'll give it a shot, but the solution may very well be that we remove support for terminals on Windows. It has proven to be more work than the team can accommodate given that none of us use Windows 10.

@blink1073 blink1073 removed this from the Future milestone Sep 12, 2018
@blink1073 blink1073 added this to the 1.0 milestone Sep 12, 2018
@dhirschfeld
Copy link
Member

@dhirschfeld dhirschfeld commented Sep 12, 2018

Sounds a bit like #5065? Which has already been fixed.

I've currently got docker build problem so can't confirm for sure. Anecdotally, I definitely had problems with terminals/notebooks restarting but haven't seen them more recently.

@blink1073
Copy link
Member

@blink1073 blink1073 commented Sep 12, 2018

Ah, I see the update now that it is not just a Windows problem.

@blink1073
Copy link
Member

@blink1073 blink1073 commented Sep 12, 2018

Thinking out loud: we need to ask the server if the terminal is still alive before reconnecting the socket automatically.

@t-makaro
Copy link
Author

@t-makaro t-makaro commented Sep 12, 2018

@dhirschfeld This is still an issue for me in 0.34.9

@afshin afshin assigned afshin and unassigned blink1073 Sep 13, 2018
@t-makaro t-makaro changed the title Trouble shutting down terminals on windows. Trouble shutting down terminals Oct 10, 2018
@gdlmx
Copy link

@gdlmx gdlmx commented Nov 6, 2018

The source of this problem comes from the class terminado.NamedTermManager, where its method get_terminal tries to create a new terminal instance when the "named terminal" doesn't exist. Below is the events timeline:

  1. User types exit in the shell.
  2. The shell process exits and then the NamedTermManager instance destroys the corresponding object in self.terminals (handled by on_eof).
  3. Server closes the websocket connection.
  4. Client tries to reconnect and send a GET request to /terminals/websocket/1.
  5. To handle this request, the get_terminal method from NamedTermManager is called and, as a result, a new terminal is created automatically.

gdlmx added a commit to gdlmx/notebook that referenced this issue Nov 6, 2018
This fixes an error that a terminal creation is triggered by a websocket opening request,
which is the default behavior of `terminado.NamedTermManager` if the client requests 
a nonexistent websocket. The error prevent a terminal from being shut down in 
JupyterLab as reported in this [issue](jupyterlab/jupyterlab#5061).
gdlmx added a commit to gdlmx/notebook that referenced this issue Nov 7, 2018
This patch prevents creation of a new terminal when handling websocket handshaking request. The default behavior of `terminado.NamedTermManager` is to automatically start a new terminal in response to a websocket handshake, which prevents a terminal from being properly shut down in JupyterLab as reported in this [issue](jupyterlab/jupyterlab#5061).
@blink1073 blink1073 assigned blink1073 and unassigned afshin May 11, 2019
@gdlmx
Copy link

@gdlmx gdlmx commented May 15, 2019

Thanks for implementing the dispose-on-disconnect feature. However, this issue #5061 is not yet fixed completely. There's still an edge case:

  1. Connect to the same terminal instance from two computers, PC A and PC B
  2. Disconnect PC A (shutdown the WiFi) while the terminal is still opened;
  3. Shutdown the terminal on PC B;
  4. Reconnect PC A

Then the dead terminal will come back to live.

In order to close a terminal instance cleanly, all websockets to it should remain connected at the time of shutdown. That seems to be a very strong requirement.

@gdlmx
Copy link

@gdlmx gdlmx commented May 15, 2019

I've tested it on jupyterlab:master after the merge of blink1073:terminal-zombies. Below are the logs:

Step 1:

[I 2019-05-15 10:50:09.160 SingleUserLabApp management:320] New terminal with automatic name: 1
[I 2019-05-15 10:50:09.163 SingleUserLabApp log:174] 200 POST /hub/api/terminals?1557909942729 (user@::ffff:192.168.0.1) 52.64ms
[I 2019-05-15 10:50:09.186 SingleUserLabApp log:174] 101 GET /hub/terminals/websocket/1?token=[secret] (user@::ffff:192.168.0.1) 3.40ms
TermSocket.open: 1
TermSocket.open: Opened 1

Step 2 & 3:

Websocket closed
[I 2019-05-15 10:50:43.007 SingleUserLabApp management:185] EOF on FD 16; stopping reading
[I 2019-05-15 10:50:43.108 SingleUserLabApp management:338] Terminal 1 closed
Websocket closed

Step 4:

[I 2019-05-15 10:50:55.127 SingleUserLabApp log:174] 101 GET /hub/terminals/websocket/1?token=[secret] (user@::ffff:192.168.0.1) 2.52ms
TermSocket.open: 1
[I 2019-05-15 10:50:55.128 SingleUserLabApp management:302] New terminal with specified name: 1
TermSocket.open: Opened 1

@blink1073
Copy link
Member

@blink1073 blink1073 commented May 15, 2019

@gdlmx, fair point. Reopening and marking as Future.

@blink1073 blink1073 reopened this May 15, 2019
@blink1073 blink1073 removed this from the 1.0 milestone May 15, 2019
@blink1073 blink1073 added this to the Future milestone May 15, 2019
@vidartf
Copy link
Member

@vidartf vidartf commented May 16, 2019

I haven't followed this too closely, but for reference: Since tornado 4, the websocket close event sets attributes about whether the close was a clean shutdown (http://www.tornadoweb.org/en/stable/websocket.html#tornado.websocket.WebSocketHandler.on_close).

@blink1073
Copy link
Member

@blink1073 blink1073 commented May 16, 2019

Nice catch, we can look at that value and handle it if we haven't gotten a disconnect message already. As for the reconnect, we'd need to poll the rest API prior to each attempt to determine whether to try and open the websocket again , checking if the server still thinks we are running.

@blink1073 blink1073 removed their assignment Aug 21, 2019
@afauroux
Copy link

@afauroux afauroux commented Apr 20, 2020

I observed a similar behavior on linux, with jupyter lab version 1.2.6 and python 3.81.

  • Linux version:
LSB Version:    :base-4.0-amd64:base-4.0-noarch:core-4.0-amd64:core-4.0-noarch:graphics-4.0-amd64:graphics-4.0-noarch:printing-4.0-amd64:printing-4.0-noarch
Distributor ID: CentOS
Description:    CentOS release 6.5 (Final)
Release:        6.5
Codename:       Final
  • python 3.8.1 and jupyter lab 1.2.6 with no extensions and no extension manager (no internet connection and nodejs not installed on the machine)

  • Observed behavior:
    When terminal tabs are closed, or when terminal are closed from the session view, they often reopen immediately. Which makes them impossible to close unless jupyter lab is restarted.

  • A quick look at the log shows an EOF error:

TermSocket.open: 3
TermSocket.open: Opened 3
Websocket closed
Websocket closed
Websocket closed
[I 17:55:00.204 LabApp] EOF on FD 23; stopping reading
[I 17:55:00.305 LabApp] Terminal 3 closed
Websocket closed
[I 17:59:44.409 LabApp] New terminal with automatic name: 3
TermSocket.open: 3
TermSocket.open: Opened 3

I am new here, so I am not sure what information could be useful or if this bug report is still relevant. Please let me know if I should give more details.

blink1073 pushed a commit to jupyter/notebook that referenced this issue Jun 7, 2020
This patch prevents creation of a new terminal when handling websocket handshaking request. The default behavior of `terminado.NamedTermManager` is to automatically start a new terminal in response to a websocket handshake, which prevents a terminal from being properly shut down in JupyterLab as reported in this [issue](jupyterlab/jupyterlab#5061).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

9 participants