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

nil deref error in treemacs when explain-pause-mode is active (root cause: process-filter wrap nil callback) #44

Closed
parsoj opened this issue Jun 18, 2020 · 5 comments · Fixed by #45
Labels
bug Something isn't working has-repro Has a repro case

Comments

@parsoj
Copy link

parsoj commented Jun 18, 2020

Opening a treemacs buffer, and calling treemacs-toggle-node on one of the folders triggers a (void-function nil) error

Debugger entered--Lisp error: (void-function nil)
  nil(#<process Process Future<1>> "finished\n")
  apply(nil (#<process Process Future<1>> "finished\n"))
  explain--measure-function(nil (#<process Process Future<1>> "finished\n") (nil sentinel-filter treemacs-TAB-action) explain-pause-log-all-process-io)
  #f(compiled-function (&rest callback-args) #<bytecode -0x1bdc34496122d242>)(#<process Process Future<1>> "finished\n")
  accept-process-output(#<process Process Future<1>>)
  pfuture-await-to-finish(#<process Process Future<1>>)
  treemacs--parse-collapsed-dirs(#<process Process Future<1>>)
  treemacs--expand-dir-node(#<marker (moves after insertion) at 48 in  *Treemacs-Scoped-Buffer-#<frame emacs (main frame) 0x7f97a8063a30>*> :recursive nil)
  treemacs-toggle-node(nil)
  treemacs-TAB-action(nil)
  funcall-interactively(treemacs-TAB-action nil)
  call-interactively(treemacs-TAB-action nil nil)
  command-execute(treemacs-TAB-action)

disabling explain-pause-mode eliminates the error.

I haven't dived much deeper into this so far (need to finish unrelated work) - but figured I'd post this here to see if there is any immediate insight. Thanks for taking a look!

@lastquestion lastquestion added the bug Something isn't working label Jun 19, 2020
@lastquestion lastquestion added this to To do in Explain-pause-mode via automation Jun 19, 2020
@lastquestion
Copy link
Owner

Hi!! Thanks for the bug report. I'm 100% sure it's because I didn't read the documentation carefully enough for set-process-filter. Quote

set-process-filter process filter
This function gives process the filter function filter. If filter is nil, it gives the process the default filter, which inserts the process output into the process buffer.

Well, duh, filter is nil here, because treemacs wanted it to go into some buffer, it must have done something like this:

(set-process-filter my-process nil)

This would drop into https://github.com/lastquestion/explain-pause-mode/blob/master/explain-pause-mode.el#L1878 which drops into https://github.com/lastquestion/explain-pause-mode/blob/master/explain-pause-mode.el#L1838 which drops into https://github.com/lastquestion/explain-pause-mode/blob/master/explain-pause-mode.el#L1807, which tries to call apply nil.

I'll pull a repro case together. The fix needs to be around https://github.com/lastquestion/explain-pause-mode/blob/master/explain-pause-mode.el#L1874 and for set-process-sentinel, too.

I think this is an easy fix, it's possible to advise the code to fix it locally too but it's a bit of a pain. I'll just suggest waiting, I'll get this fixed today.

Thanks again for reporting!! I'm surprised no one else has hit this... kind of a glaring bug 🤣

@lastquestion lastquestion added the has-repro Has a repro case label Jun 21, 2020
@lastquestion lastquestion changed the title nil deref error in treemacs when explain-pause-mode is active nil deref error in treemacs when explain-pause-mode is active (root cause: process-filter wrap nil callback) Jun 21, 2020
@lastquestion
Copy link
Owner

Repro case

(setq proc (make-process
            :name "test"
            :buffer "test"
            :command '("bash")))
(set-process-filter proc nil)
(process-send-string proc "ls -al\n")

@lastquestion
Copy link
Owner

Hi!! Sorry this took longer to fix then I thought. This should fix the issue, please re-open if it doesn't!

@lastquestion
Copy link
Owner

Hi @tomfitzhenry I was going through old PRs to add repro cases to a set of integration test - a little creepy but I saw your reference 😀

with-editor is in magit and is relatively complex, the other open issue #26 is also related to with-editor. I have (I think) finally quashed #26 in the open PR #42. The bug you hit is likely to be related to that, what happened is probably the filter intercept code failed with an error and so never returned the value back out to with-editor. When #42 is merged I give it more then even odds that the bug you found will be fixed, but if not, feel free to open a bug with just very basic info, it's super appreciated if you can debug into it but not necessary :_)

@tomfitzhenry
Copy link

tomfitzhenry commented Jun 28, 2020

Hi @tomfitzhenry I was going through old PRs to add repro cases to a set of integration test - a little creepy but I saw your reference grinning

Not creepy at all! That's one of the handy parts of Github.

When #42 is merged I give it more then even odds that the bug you found will be fixed, but if not, feel free to open a bug with just very basic info, it's super appreciated if you can debug into it but not necessary :_)

I've now raised a bug at #46 .

Thanks for your great work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working has-repro Has a repro case
Projects
3 participants