Skip to content

fix: send SIGTERM to job when trying to exit#125

Merged
jarun merged 1 commit intomcchrish:masterfrom
N-R-K:term_job_on_exit
Oct 20, 2021
Merged

fix: send SIGTERM to job when trying to exit#125
jarun merged 1 commit intomcchrish:masterfrom
N-R-K:term_job_on_exit

Conversation

@N-R-K
Copy link
Copy Markdown
Collaborator

@N-R-K N-R-K commented Oct 20, 2021

@N-R-K
Copy link
Copy Markdown
Collaborator Author

N-R-K commented Oct 20, 2021

@jarun can you confirm if this fixes the :qa issue on vim? On neovim the issue doesn't happen (at least I'm not able to reproduce it), so no changes should be needed.

@N-R-K
Copy link
Copy Markdown
Collaborator Author

N-R-K commented Oct 20, 2021

And btw, this issue happens on NnnPicker as well. But I guess no one tried to do :qa while having Picker window open so it never got caught.

@jarun jarun merged commit c644145 into mcchrish:master Oct 20, 2021
@jarun
Copy link
Copy Markdown
Collaborator

jarun commented Oct 20, 2021

Yes, it's fixed.

However, I still see:

mkfifo: cannot create fifo '/tmp/vHsmzKA/0': File exists

@N-R-K N-R-K deleted the term_job_on_exit branch October 21, 2021 09:45
@N-R-K
Copy link
Copy Markdown
Collaborator Author

N-R-K commented Oct 21, 2021

However, I still see:

mkfifo: cannot create fifo '/tmp/vHsmzKA/0': File exists

Can't reproduce. Can you manually delete that file and see if the issue is reproducible on your end?

@jarun
Copy link
Copy Markdown
Collaborator

jarun commented Oct 21, 2021

It's a new file everytime.
But I noticed that I see this error if I just open the explorer (but not any new file using it).
This is thrown by mkfifo. Are you trying to create a fifo that already exists?

@N-R-K
Copy link
Copy Markdown
Collaborator Author

N-R-K commented Oct 21, 2021

This is thrown by mkfifo. Are you trying to create a fifo that already exists?

Fifo should be only created at :NnnExplorer startup.

execute 'silent !mkfifo '.s:explorer_fifo

We're creating this ourselves to avoid the sleep hack. I saw @luukvbaal do it on nnn.nvim as well.

@luukvbaal
Copy link
Copy Markdown

I put it in the setup() function so it is only run once.

@luukvbaal
Copy link
Copy Markdown

I guess you should just put it at the top of ftplugin/nnn.vim or autoload/nnn.vim idk 🤷🏻‍♂️

@N-R-K
Copy link
Copy Markdown
Collaborator Author

N-R-K commented Oct 21, 2021

We're deleting the fifo on explorer exit though. So don't think that should be the problem. I think maybe the fifo isn't getting deleted for some reason.

Will have to look deeper into this.

@jarun
Copy link
Copy Markdown
Collaborator

jarun commented Oct 21, 2021

I don't see a leftover fifo in my tmp after I quit vim.
Each time a random fifo is created and deleted when I quit vim.

After I open Explorer, with vi still open I can see the directory /tmp/vRTFHVf/. So I guess somehow we are attempting to create it twice. If nothing, can you try to make it silent?

@N-R-K
Copy link
Copy Markdown
Collaborator Author

N-R-K commented Oct 21, 2021

@jarun can try the following patch and see if that works? I think we need to create the fifo before calling s:build_window otherwise nnn might be trying to create the fifo as well.

diff --git a/autoload/nnn.vim b/autoload/nnn.vim
index a883d3e..660cb4d 100644
--- a/autoload/nnn.vim
+++ b/autoload/nnn.vim
@@ -385,11 +385,11 @@ function! nnn#explorer(...) abort
     let l:opts.ppos = { 'buf': bufnr(''), 'win': winnr(), 'tab': tabpagenr() }
     let l:On_exit = s:explorer_create_on_exit_callback(l:opts)
 
+    " create the fifo ourselves since otherwise nnn might not create it on time
+    execute 'silent !mkfifo '.s:explorer_fifo
     let l:opts.term = s:build_window(l:layout, { 'cmd': l:cmd, 'on_exit': l:On_exit })
     let b:tbuf = l:opts.term
 
-    " create the fifo ourselves since otherwise nnn might not create it on time
-    execute 'silent !mkfifo '.s:explorer_fifo
     call s:explorer_job()
 
     autocmd BufEnter <buffer> startinsert

@jarun
Copy link
Copy Markdown
Collaborator

jarun commented Oct 21, 2021

@N-R-K somehow I don't see the issue anymore so ignore it right now. If I see it again after reboot or something I will try this and update you.

Thanks a lot for checking this out and the awesome Explorer mode!!!

@N-R-K
Copy link
Copy Markdown
Collaborator Author

N-R-K commented Oct 21, 2021

@jarun I can reproduce the issue now after adding a sleep 2 after s:build_window. This means nnn is creating the fifo before us. I'll open the PR with the fix.

N-R-K added a commit to N-R-K/nnn.vim that referenced this pull request Oct 21, 2021
otherwise nnn might end up creating a fifo before us.

Closes: mcchrish#125 (comment)
@jarun
Copy link
Copy Markdown
Collaborator

jarun commented Oct 21, 2021

Awesome!

jarun pushed a commit that referenced this pull request Oct 21, 2021
otherwise nnn might end up creating a fifo before us.

Closes: #125 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants