atlas-engineer / cl-electron

Lisp Interface to Electron.
BSD 3-Clause "New" or "Revised" License
16 stars 1 forks source link

Socket deletion on `terminate` #15

Closed aadcg closed 5 months ago

aadcg commented 1 year ago

Question 1

The call to uiop:terminate-process in terminate deletes both sockets since it kills the process. Do we really need (uiop:delete-file-if-exists (lisp-socket-path interface)) in terminate? Or, in other words, could it happen that there would be no Electron process but lisp.socket exists?

Question 2

I've noticed that the following sometimes results in the error below, which means that the creation of the window is happening before launch had time to create the socket.

(progn
  (setf electron::*interface*
        (make-instance
         'electron:interface
         :electron-socket-path (uiop:xdg-runtime-dir "electron.socket")
         :lisp-socket-path (uiop:xdg-runtime-dir "lisp.socket"))) 
  (electron:launch)
  ;; (sleep 1)
  (defvar win (make-instance 'electron:browser-window)))
#<Syscall "connect" signalled error ENOENT(2) "No such file or directory" FD=10>
   [Condition of type IOLIB/SYSCALLS:ENOENT]

Restarts:
 0: [RETRY] Retry SLY mREPL evaluation request.
 1: [*ABORT] Return to SLY's top level.
 2: [ABORT] abort thread (#<THREAD tid=17882 "sly-channel-1-mrepl-remote-1" RUNNING {1008901AB3}>)

Backtrace:
 0: ((LAMBDA NIL :IN IOLIB/SOCKETS:CONNECT))
      [No Locals]
 1: (IOLIB/SOCKETS::CALL-WITH-SOCKET-TO-WAIT-CONNECT #<active local stream socket, unconnected {100D3F62C3}> #<FUNCTION (LAMBDA NIL :IN IOLIB/SOCKETS:CONNECT) {100D57525B}> T)
      Locals:
        SOCKET = #<active local stream socket, unconnected {100D3F62C3}>
        THUNK = #<FUNCTION (LAMBDA () :IN IOLIB/SOCKETS:CONNECT) {100D57525B}>
        TIMEOUT = NIL
        WAIT = T
 2: ((:METHOD IOLIB/SOCKETS:CONNECT (IOLIB/SOCKETS:LOCAL-SOCKET IOLIB/SOCKETS:LOCAL-ADDRESS)) #<active local stream socket, unconnected {100D3F62C3}> #<Unix socket address: "/run/user/1000/electron.socket..
      Locals:
        IOLIB/SOCKETS:ADDRESS = #<Unix socket address: "/run/user/1000/electron.socket">
        IOLIB/SOCKETS:SOCKET = #<active local stream socket, unconnected {100D3F62C3}>
        IOLIB/SOCKETS::WAIT = T
 3: (IOLIB/SOCKETS::%%INIT-SOCKET/LOCAL-STREAM-ACTIVE #<active local stream socket, unconnected {100D3F62C3}> NIL "/run/user/1000/electron.socket")
      Locals:
        LOCAL-FILENAME = NIL
        REMOTE-FILENAME = "/run/user/1000/electron.socket"
        SOCKET = #<active local stream socket, unconnected {100D3F62C3}>
 4: (SEND-MESSAGE #<BROWSER-WINDOW {100D3F1B03}> "ID573 = new BrowserWindow();")
      Locals:
        MESSAGE = "ID573 = new BrowserWindow();"
        IOLIB/SOCKETS:SOCKET = #<active local stream socket, unconnected {100D3F62C3}>
        TARGET = #<BROWSER-WINDOW {100D3F1B03}>
 5: ((:METHOD INITIALIZE-INSTANCE :AFTER (BROWSER-WINDOW)) #<BROWSER-WINDOW {100D3F1B03}> :OPTIONS "") [fast-method]
      Locals:
        BROWSER-WINDOW = #<BROWSER-WINDOW {100D3F1B03}>
        SB-FORMAT::FORMAT-ARG2 = ""
 6: ((SB-PCL::EMF INITIALIZE-INSTANCE) #<unused argument> #<unused argument> #<BROWSER-WINDOW {100D3F1B03}>)
 7: ((:METHOD MAKE-INSTANCE (CLASS)) #<STANDARD-CLASS ELECTRON:BROWSER-WINDOW>) [fast-method]
...

CC @Ambrevar @jmercouris

(@jmercouris I've deleted your first reply since I've edited my top post heavily.)

jmercouris commented 1 year ago

Depends, is it part of the contract in the API that upon termination of a process the sockets will be deleted? If it is, then we don't, otherwise we do.

aadcg commented 1 year ago

is it part of the contract in the API that upon termination of a process the sockets will be deleted?

This is not even related to Electron. If you start a process, say electron /path/to/server.js /run/user/1000/electron.socket /run/user/1000/lisp.socket and kill it (by sending the KILL signal), both sockets are deleted.

Hence the question: is there a situation when there would be no Electron process, but lisp.socket would exist? If it does, we should keep everything as is. It it doesn't, then the call to (uiop:delete-file-if-exists (lisp-socket-path interface)) in terminate seems extra to me.

jmercouris commented 1 year ago

I am asking if killing a process ALWAYS results in the socket being deleted by design. If it is a Linux specific quirk, or if it is part of the posix spec.

aadcg commented 1 year ago

Good question. I'm not sure. Maybe it wouldn't be a bad idea to delete both of them explicitly just in case.

Ambrevar commented 1 year ago

It has nothing to do with Posix / Linux: socket must be removed by the process. We do this with iolib's iolib:with-open-socket.

The uiop:delete-file-if-exists (lisp-socket-path interface)) is technically not necessary, but it does not hurt as a safe-guard in case we forget to handle errors properly in the body.

As for question 2, this is an issue indeed, we should have a mean to wait for the server to be ready. Off the top of my head, I'd say that electron should write a special message to the Lisp socket or something like that. It should be part of our API / contract that this message is consumed.

aadcg commented 1 year ago

It has nothing to do with Posix / Linux: socket must be removed by the process.

Are you sure? When I send the KILL signal to the Electron process (from Emacs' proced), the sockets are deleted. IOlib isn't involved in it.

Thanks for the clarifications.

Question 2 is critical but, for the time being, I can just add temporary calls to sleep so that I continue the Nyxt - Electron integration activities.

Ambrevar commented 1 year ago

Are you sure? When I send the KILL signal to the Electron process (from Emacs' proced), the sockets are deleted. IOlib isn't involved in it.

Are you sure back? :) I just tried this:

kill -9 4775

(PID of the SBCL image) and the sockets were not cleaned up.

aadcg commented 1 year ago

I meant killing the Electron process, not the SBCL process.

Ambrevar commented 9 months ago

Oh yes, of course this works since the Lisp side handles killed Electron processes, but what I meant is that's only one side of the issue of "cleaning up sockets".

Anyways, I believe the answer to Question 1 is still as was said before:

The uiop:delete-file-if-exists (lisp-socket-path interface)) is technically not necessary, but it does not hurt as a safe-guard in case we forget to handle errors properly in the body.

aadcg commented 5 months ago

Commit 98fc836 puts an end to question 1.

Commit e647943dac358f97f3a27ab4a49dad3625c5c471 should have fixed question 2.