ShahanaFarooqui / clnrest

clnrest
MIT License
3 stars 0 forks source link

Maybe the server should be stopped when we stop the plugin #1

Closed tonyaldon closed 1 year ago

tonyaldon commented 1 year ago

Hi Shahana,

I tried clnrest and when I was trying to start it with the option rest_protocol=http, the server didn't want to run at http://127.0.0.1:3010. Here is the log:

2023-06-28T14:28:41.362Z DEBUG   plugin-manager: started(364633) /tmp/clnrest/clnrest/clnrest.py
2023-06-28T14:28:41.763Z INFO    plugin-clnrest.py: Server is starting at https://127.0.0.1:3010
2023-06-28T14:28:41.764Z INFO    plugin-clnrest.py: Certificate not found at /tmp/l1-regtest/regtest. Generating a new certificate!
2023-06-28T14:28:41.790Z INFO    plugin-clnrest.py: Certificate Generated!
2023-06-28T14:28:41.790Z INFO    plugin-clnrest.py: Certs Path: /tmp/l1-regtest/regtest
2023-06-28T14:28:41.799Z UNUSUAL plugin-clnrest.py: [2023-06-28 16:28:41 +0200] [364634] [INFO] Starting gunicorn 20.1.0
2023-06-28T14:28:41.799Z UNUSUAL plugin-clnrest.py: [2023-06-28 16:28:41 +0200] [364634] [INFO] Listening at: https://127.0.0.1:3010 (364634)
2023-06-28T14:28:41.799Z UNUSUAL plugin-clnrest.py: [2023-06-28 16:28:41 +0200] [364634] [INFO] Using worker: sync
2023-06-28T14:28:41.803Z UNUSUAL plugin-clnrest.py: [2023-06-28 16:28:41 +0200] [364641] [INFO] Booting worker with pid: 364641
2023-06-28T14:28:41.830Z UNUSUAL plugin-clnrest.py: [2023-06-28 16:28:41 +0200] [364642] [INFO] Booting worker with pid: 364642
2023-06-28T14:28:41.838Z UNUSUAL plugin-clnrest.py: [2023-06-28 16:28:41 +0200] [364643] [INFO] Booting worker with pid: 364643
2023-06-28T14:28:41.843Z UNUSUAL plugin-clnrest.py: [2023-06-28 16:28:41 +0200] [364644] [INFO] Booting worker with pid: 364644
2023-06-28T14:29:31.697Z DEBUG   gossipd: seeker: no peers, waiting
2023-06-28T14:30:31.753Z DEBUG   gossipd: seeker: no peers, waiting
2023-06-28T14:31:31.809Z DEBUG   gossipd: seeker: no peers, waiting
2023-06-28T14:32:15.691Z INFO    plugin-clnrest.py: Killing plugin: stopped by lightningd via RPC
2023-06-28T14:32:31.865Z DEBUG   gossipd: seeker: no peers, waiting
2023-06-28T14:33:28.612Z DEBUG   plugin-manager: started(364982) /tmp/clnrest/clnrest/clnrest.py
2023-06-28T14:33:28.990Z INFO    plugin-clnrest.py: Server is starting at http://127.0.0.1:3010
2023-06-28T14:33:29.007Z UNUSUAL plugin-clnrest.py: [2023-06-28 16:33:29 +0200] [364983] [INFO] Starting gunicorn 20.1.0
2023-06-28T14:33:29.008Z UNUSUAL plugin-clnrest.py: [2023-06-28 16:33:29 +0200] [364983] [ERROR] Connection in use: ('127.0.0.1', 3010)
2023-06-28T14:33:29.008Z UNUSUAL plugin-clnrest.py: [2023-06-28 16:33:29 +0200] [364983] [ERROR] Retrying in 1 second.
2023-06-28T14:33:30.009Z UNUSUAL plugin-clnrest.py: [2023-06-28 16:33:30 +0200] [364983] [ERROR] Connection in use: ('127.0.0.1', 3010)
2023-06-28T14:33:30.010Z UNUSUAL plugin-clnrest.py: [2023-06-28 16:33:30 +0200] [364983] [ERROR] Retrying in 1 second.
2023-06-28T14:33:31.011Z UNUSUAL plugin-clnrest.py: [2023-06-28 16:33:31 +0200] [364983] [ERROR] Connection in use: ('127.0.0.1', 3010)
2023-06-28T14:33:31.011Z UNUSUAL plugin-clnrest.py: [2023-06-28 16:33:31 +0200] [364983] [ERROR] Retrying in 1 second.

This is because when we stop the plugin, the server is not stopped and so the port 3010 is still used.

You can check this by running the following (assuming we have lightning and clnrest repository cloned locally in the current directory):

$ source lightning/contrib/startup_regtest.sh
$ start_ln
$ l1-cli plugin start $(pwd)/clnrest/clnrest.py
...
$ netstat -ln | rg 3010
tcp        0      0 127.0.0.1:3010          0.0.0.0:*               LISTEN
$ l1-cli plugin stop clnrest.py
{
   "command": "stop",
   "result": "Successfully stopped clnrest.py."
}
$ netstat -ln | rg 3010
tcp        0      0 127.0.0.1:3010          0.0.0.0:*               LISTEN

Maybe the server should be stopped when we stop the plugin. What do you think?

If you think it is a reasonable idea, this can be done by subscribing to the shutdown notification adding something like this to clnrest.py:

import sys

@plugin.subscribe("shutdown")
def stop_server(plugin,**kwargs):
    from utilities.shared import REST_PORT
    jobs[REST_PORT].terminate()
    sys.exit(0)
ShahanaFarooqui commented 1 year ago

@tonyaldon Thank you so much for testing the plugin. Much appreciated.

This issue was actually a side effect of some other bug related to my notifications/stream testing. The merged the working code in the master now. It should not be happening anymore.

Closing the issue now but please feel free to comment if you still see the same behaviour.