Send kernel shutdown request manually on beforeunload #612

jtpio · 2020-05-20T13:04:31Z

This manually sends a DELETE /api/kernels/<kernel-id> with async = false

https://www.w3schools.com/js/js_ajax_http_send.asp

vidartf · 2020-05-22T10:55:10Z

share/jupyter/voila/templates/default/static/main.js

@@ -40,7 +40,9 @@ require(['static/voila'], function(voila) {
        async function init() {
            // it seems if we attach this to early, it will not be called
            window.addEventListener('beforeunload', function (e) {
-                kernel.shutdown();
+                const xhttp = new XMLHttpRequest();
+                xhttp.open("DELETE", `/api/kernels/${kernel.id}`, false);


Note that if end up deciding that this is the best way, we would need to ensure we get/set all the appropriate headers and cookies.

Which is why using sendBeacon instead sounds interesting 👍 (linking to #528 for more context)

There are more things than just cookies though, i.e. a bunch of headers:
https://github.com/jupyterlab/jupyterlab/blob/64425baacfba677b75efaf05cfb825a82264230d/packages/services/src/serverconnection.ts#L236-L282

How many of those are needed for the DELETE request is a bit unknown though.

I think it might be possible to do a workaround by using a ServerConnection with our own settings that override fetch such that it instead uses a sync XHTTPRequest? It would require "translating" the request first though.

There are more things than just cookies though, i.e. a bunch of headers:

Indeed. And these are not picked up by sendBeacon.

jtpio · 2020-05-22T15:24:14Z

Experimented a bit with sendBeacon locally. The following diff exposes a separate POST endpoint to shut the kernels down (following the idea in #528):

diff --git a/share/jupyter/voila/templates/default/static/main.js b/share/jupyter/voila/templates/default/static/main.js
index ae5baef..2e1ce4e 100644
--- a/share/jupyter/voila/templates/default/static/main.js
+++ b/share/jupyter/voila/templates/default/static/main.js
@@ -40,9 +40,7 @@ require(['static/voila'], function(voila) {
         async function init() {
             // it seems if we attach this to early, it will not be called
             window.addEventListener('beforeunload', function (e) {
-                const xhttp = new XMLHttpRequest();
-                xhttp.open("DELETE", `/api/kernels/${kernel.id}`, false);
-                xhttp.send();
+                window.navigator.sendBeacon('/api/voila/shutdown/' + kernel.id);
                 kernel.dispose();
             });
             await widgetManager.build_widgets();
diff --git a/voila/app.py b/voila/app.py
index 17d9f33..067165d 100644
--- a/voila/app.py
+++ b/voila/app.py
@@ -55,6 +55,7 @@ from ipython_genutils.py3compat import getcwd

 from .paths import ROOT, STATIC_ROOT, collect_template_paths
 from .handler import VoilaHandler
+from .stophandler import VoilaStopHandler
 from .treehandler import VoilaTreeHandler
 from ._version import __version__
 from .static_file_handler import MultiStaticFileHandler, WhiteListFileHandler
@@ -505,6 +506,9 @@ class Voila(Application):
                  }),
             ])

+        handlers.append(
+            (url_path_join(self.server_url, r'/api/voila/shutdown/(.*)'), VoilaStopHandler)
+        )
         self.app.add_handlers('.*$', handlers)
         self.listen()

diff --git a/voila/stophandler.py b/voila/stophandler.py
new file mode 100644
index 0000000..b8a677d
--- /dev/null
+++ b/voila/stophandler.py
@@ -0,0 +1,10 @@
+import tornado
+
+from jupyter_server.base.handlers import APIHandler
+
+class VoilaStopHandler(APIHandler):
+    @tornado.web.authenticated
+    async def post(self, kernel_id):
+        await self.kernel_manager.shutdown_kernel(kernel_id)
+        self.set_status(204)
+        self.finish()

Although this seems to be fine in voila standalone locally, we are indeed missing the authorization headers (when uses as a server extension for example).

jtpio · 2020-05-22T15:43:21Z

2ea00a6 and deda754 seem to improve things slightly, mostly in the case of the preview extension.

Before:

After:

However it doesn't prevent a user to hit refresh very quickly (#55). In that case not all the requests make it and kernels are not shut down.

It also doesn't seem to be effective when the preview panel is closed.

jtpio · 2020-05-25T14:32:20Z

@vidartf let me know what you think if you get to try it locally.

It should already be a slight improvement. Although a more robust fix would probably be to detect when a connection is closed on the server instead, and shut down the corresponding kernel (or at least make that behavior configurable).

maartenbreddels · 2020-05-26T11:33:01Z

Is this something we'd like to test with say puppeteer?

Also, I think long term we should upstream this into jupyterlab right?

maartenbreddels · 2020-05-26T11:33:31Z

Although a more robust fix would probably be to detect when a connection is closed on the server instead, and shut down the corresponding kernel (or at least make that behavior configurable).

Culling set to 0.01 minutes?

jtpio · 2020-05-26T12:27:59Z

Is this something we'd like to test with say puppeteer?

It would be very interesting to check how this is handled with puppeteer. Whether the request is correctly fired or not.

Also, I think long term we should upstream this into jupyterlab right?

You mean the option to send requests synchronously?

Culling set to 0.01 minutes?

That could work. Otherwise wouldn't it be possible to know on the server when a client has disconnected? Because this should correspond to a websocket connection closed.

vidartf · 2020-06-01T11:39:11Z

Although a more robust fix would probably be to detect when a connection is closed on the server instead, and shut down the corresponding kernel (or at least make that behavior configurable).

While I believe there might be a technical solution possible here, I don't know anything about how hard it would be to code it and get a PR accepted.

Culling set to 0.01 minutes?

This would cause a network glitch (the train driving through a tunnel) to kill your kernel, so probably not ideal, but yes, having an aggressive culling time seems to be the current best option.

Regarding the most robust solution, I saw someone recommend using a fetch request with the keepalive flag set to true. I'm reading more on that now.

vidartf · 2020-06-01T12:12:43Z

Regarding the keepalive flag, I think this would be the most ideal thing for us (it was designed for our use case, and it is often what sendBeacon uses under the hood). The only hiccup is that safari doesn't seem to support it (according to MDN, and here https://bugs.webkit.org/show_bug.cgi?id=168865).

maartenbreddels · 2020-09-07T12:55:23Z

I think the current solution is an improvement (not a total fix though). I'd be happy to merge this, until we find a better solution, possibly in the jupyterlab org/repo?
Does that sound like a good idea for everyone?

jtpio · 2020-09-08T09:01:25Z

Does that sound like a good idea for everyone?

Sounds good 👍

github-actions · 2021-09-16T14:03:43Z

Benchmark report

The execution time (in milliseconds) are grouped by test file, test type and browser.
For each case, the following values are computed: min <- [1st quartile - median - 3rd quartile] -> max.

Results table

Test file	basics.ipynb	bqplot.ipynb	dashboard.ipynb	gridspecLayout.ipynb	interactive.ipynb	ipympl.ipynb	ipyvolume.ipynb	multiple_widgets.ipynb	query-strings.ipynb
Render
`chromium`
actual	3434 <- [3524 - 3631 - 3740] -> 3996	3088 <- [3301 - 3470 - 3573] -> 3678	3094 <- [3360 - 3386 - 3394] -> 3430	4098 <- [4194 - 4261 - 4361] -> 4413	2387 <- [2488 - 2493 - 2530] -> 2532	3412 <- [3431 - 3485 - 3499] -> 3751	11916 <- [20064 - 21852 - 22123] -> 22312	12978 <- [13017 - 13334 - 13451] -> 13477	1562 <- [1776 - 1842 - 1843] -> 1867
expected	3379 <- [3442 - 3517 - 3701] -> 3876	2976 <- [3227 - 3321 - 3421] -> 3604	3608 <- [3623 - 3709 - 3793] -> 3825	4453 <- [4453 - 4523 - 4661] -> 4748	2559 <- [2655 - 2656 - 2660] -> 2674	3982 <- [4079 - 4213 - 4356] -> 4743	12183 <- [18509 - 19553 - 20811] -> 21515	15319 <- [15660 - 15796 - 15912] -> 16056	1517 <- [1920 - 1997 - 2103] -> 2113

❗ Test metadata have changed

--- /dev/fd/63	2021-09-16 14:03:16.549200147 +0000
+++ /dev/fd/62	2021-09-16 14:03:16.549200147 +0000
@@ -8,27 +8,27 @@
   },
   "systemInformation": {
     "cpu": {
-      "brand": "Xeon® Platinum 8171M",
+      "brand": "Xeon® E5-2673 v3",
       "cache": {
         "l1d": 65536,
         "l1i": 65536,
-        "l2": 2097152,
-        "l3": 36700160
+        "l2": 524288,
+        "l3": 31457280
       },
       "cores": 2,
       "family": "6",
-      "flags": "fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush mmx fxsr sse sse2 ss ht syscall nx pdpe1gb rdtscp lm constant_tsc rep_good nopl xtopology cpuid pni pclmulqdq ssse3 fma cx16 pcid sse4_1 sse4_2 movbe popcnt aes xsave avx f16c rdrand hypervisor lahf_lm abm 3dnowprefetch invpcid_single pti fsgsbase bmi1 hle avx2 smep bmi2 erms invpcid rtm mpx avx512f avx512dq rdseed adx smap clflushopt avx512cd avx512bw avx512vl xsaveopt xsavec xsaves md_clear",
+      "flags": "fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush mmx fxsr sse sse2 ss ht syscall nx pdpe1gb rdtscp lm constant_tsc rep_good nopl xtopology cpuid pni pclmulqdq ssse3 fma cx16 pcid sse4_1 sse4_2 movbe popcnt aes xsave avx f16c rdrand hypervisor lahf_lm abm invpcid_single pti fsgsbase bmi1 avx2 smep bmi2 erms invpcid xsaveopt md_clear",
       "governor": "",
       "manufacturer": "Intel®",
-      "model": "85",
+      "model": "63",
       "physicalCores": 2,
       "processors": 1,
       "revision": "",
       "socket": "",
-      "speed": 2.6,
+      "speed": 2.4,
       "speedMax": null,
       "speedMin": null,
-      "stepping": "4",
+      "stepping": "2",
       "vendor": "GenuineIntel",
       "virtualization": false,
       "voltage": ""
@@ -42,11 +42,11 @@
       "codename": "Focal Fossa",
       "codepage": "UTF-8",
       "distro": "Ubuntu",
-      "kernel": "5.8.0-1041-azure",
+      "kernel": "5.8.0-1040-azure",
       "logofile": "ubuntu",
       "platform": "linux",
       "release": "20.04.3 LTS",
-      "serial": "faac5b9e00cf4ae3b2d2c013e98d933a",
+      "serial": "cfc067bfcb844f35865e279a1b0e66c5",
       "servicepack": "",
       "uefi": false
     }

jtpio · 2021-09-16T15:25:37Z

To test this branch on Binder: https://mybinder.org/v2/gh/jtpio/voila/shutdown-page-closed?urlpath=voila/tree

jtpio mentioned this pull request May 22, 2020

Kernel not shut down on page reload in Firefox #528

Open

vidartf reviewed May 22, 2020

View reviewed changes

jtpio force-pushed the shutdown-page-closed branch from deda754 to eeb4098 Compare September 8, 2020 09:00

jtpio marked this pull request as ready for review September 8, 2020 09:01

jtpio mentioned this pull request Mar 17, 2021

Kernals from Unused Views #849

Open

jtpio added 2 commits September 16, 2021 15:40

Send kernel shutdown request manually on beforeunload

2e6d98c

Send X-XSRFToken header

074618f

jtpio added the bug Something isn't working label Sep 16, 2021

jtpio added this to the 0.2.x milestone Sep 16, 2021

jtpio force-pushed the shutdown-page-closed branch from eeb4098 to 074618f Compare September 16, 2021 13:52

jtpio requested a review from martinRenou September 16, 2021 13:53

martinRenou merged commit 1737b86 into voila-dashboards:main Sep 17, 2021

jtpio deleted the shutdown-page-closed branch September 17, 2021 08:51

jtpio mentioned this pull request Sep 20, 2021

Use sendBeacon to send kernel shutdown request. #972

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Send kernel shutdown request manually on beforeunload #612

Send kernel shutdown request manually on beforeunload #612

jtpio commented May 20, 2020 •

edited

Loading

vidartf May 22, 2020

jtpio May 22, 2020 •

edited

Loading

vidartf May 22, 2020

jtpio May 22, 2020 •

edited

Loading

jtpio commented May 22, 2020

jtpio commented May 22, 2020

jtpio commented May 25, 2020

maartenbreddels commented May 26, 2020

maartenbreddels commented May 26, 2020

jtpio commented May 26, 2020

vidartf commented Jun 1, 2020

vidartf commented Jun 1, 2020

maartenbreddels commented Sep 7, 2020

jtpio commented Sep 8, 2020

github-actions bot commented Sep 16, 2021

jtpio commented Sep 16, 2021

Send kernel shutdown request manually on beforeunload #612

Send kernel shutdown request manually on beforeunload #612

Conversation

jtpio commented May 20, 2020 • edited Loading

vidartf May 22, 2020

Choose a reason for hiding this comment

jtpio May 22, 2020 • edited Loading

Choose a reason for hiding this comment

vidartf May 22, 2020

Choose a reason for hiding this comment

jtpio May 22, 2020 • edited Loading

Choose a reason for hiding this comment

jtpio commented May 22, 2020

jtpio commented May 22, 2020

jtpio commented May 25, 2020

maartenbreddels commented May 26, 2020

maartenbreddels commented May 26, 2020

jtpio commented May 26, 2020

vidartf commented Jun 1, 2020

vidartf commented Jun 1, 2020

maartenbreddels commented Sep 7, 2020

jtpio commented Sep 8, 2020

github-actions bot commented Sep 16, 2021

Benchmark report

jtpio commented Sep 16, 2021

jtpio commented May 20, 2020 •

edited

Loading

jtpio May 22, 2020 •

edited

Loading

jtpio May 22, 2020 •

edited

Loading