universal-ctags / citre

A superior code reading & auto-completion tool with pluggable backends.
GNU General Public License v3.0
320 stars 26 forks source link

citre-global-dbpath never terminates #150

Closed ST-Saint closed 3 months ago

ST-Saint commented 1 year ago
citre

If find GNU Emacs ctags on PATH, citre-global-dbpath will not terminate

ST-Saint commented 1 year ago

if the .tags file is tainted by GNU ctags, it also hangs citre-global-dbpath

ST-Saint commented 1 year ago

Another typical scenario is open an org file with c source blocks

ST-Saint commented 1 year ago

minimal config

;; -*- lexical-binding: t -*-

(setq package-archives '(("melpa" . "https://melpa.org/packages/")
                         ("org" . "https://orgmode.org/elpa/")
                         ("elpa" . "https://elpa.gnu.org/packages/")))

(package-initialize)

(unless package-archive-contents
  (package-refresh-contents))

(unless (package-installed-p 'use-package)
  (package-install 'use-package))

(require 'use-package)
(setq use-package-always-ensure t)

(use-package dashboard
  :config
  (dashboard-setup-startup-hook))

(use-package magit-todos)

(use-package org
  :init
  (setq org-agenda-files '("/home/org/todo.org"))
  )
(use-package citre
  :config
  (require 'citre-config))

"/home/org/todo.org" seems have something to do with emacsql. Using the another file with the same content doesn't work.

And, if debugged step-by-step, the citre-global-dbpath could exit normally (see the following figure).

debug-on-entry:

citre1

profile-cpu

citre
heartnheart commented 1 year ago

I encountered same issue on GNU Emacs 30.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.33, cairo version 1.16.0) of 2023-06-05 within WSL2

It seems that emacs got stuck in an infinite loop at the line (while (not finished) (accept-process-output)) in (citre-get-output-lines). The value of variable finished should be changed to t in async process callback as soon as it receives a non-output status. However, for some reason, the async subprocess sentinel does not trigger until I press C-g to break the loop.

I also tested it by calling (citre-get-output-lines (list "global" "--print-dbpath")) interactively, and it worked without issue.

As a workaround for now, I force (citre-global-dbpath) to use (shell-command-to-string) instead of (citre-global--get-output-lines '("--print-dbpath"))

(defun citre-global-dbpath (&optional dir)
  "Get global database path.
This is the directory containing the GTAGS file.  When DIR is
non-nil, find database of that directory, otherwise find the
database of current directory.

When the global program is not found on the machine, return nil
as it is needed to get the database path."
  (when (citre-executable-find (or citre-global-program "global") t)
    (pcase citre--global-dbpath
      ('none nil)
      ((and val (pred stringp) (pred citre-dir-exists-p)) val)
      (_ (let ((default-directory (or default-directory dir)))
           (condition-case nil
               (setq citre--global-dbpath
                     ;; change this line 
                     ;;     (car (citre-global--get-output-lines '("--print-dbpath")))
                     ;; to
                     (string-trim (shell-command-to-string (concat citre--global-dbpath " --print-dbpath")))
                     )
             (error (setq citre--global-dbpath 'none)
                    nil)))))))

I hope this helps!

AmaiKinono commented 1 year ago

@heartnheart What happens if you change

(while (not finished) (accept-process-output))

to

(while (eq (process-status proc) 'run) (accept-process-output))

? Thanks.

heartnheart commented 11 months ago

@heartnheart What happens if you change

(while (not finished) (accept-process-output))

to

(while (eq (process-status proc) 'run) (accept-process-output))

? Thanks.

@AmaiKinono Sorry for the late reply. But I can't reproduce until today.

Changing to

(while (eq (process-status proc) 'run) (accept-process-output))

fix the problem!

Thanks!

AmaiKinono commented 10 months ago

@heartnheart I'm very sorry for the delay.

The fix I suggested works for you and me, but it causes the tests to fail randomly on the CI. I guess the problem is deeply wired with the underlying OS so it works for some, fails for others.

Here's another version of citre-get-output-lines that I think may fix the problem. Could you or @ST-Saint try it?

(defun citre-get-output-lines (cmd)
  (let* ((result nil)
         (err-msg nil)
         (finish-status nil)
         (callback
          (lambda (status msg)
            (pcase status
              ('output (setq result
                             (nconc result (split-string msg "\n" t))))
              (0 (setq finish-status 'success))
              ((and s (pred integerp))
               (setq finish-status 'fail
                     err-msg (format "Process %s exits %s:\n%s"
                                     (car cmd) s msg)))
              ('signal (setq finish-status 'fail))
              (s (setq finish-status 'fail
                       err-msg (format "Abnormal status of process %s:\n%s"
                                       (car cmd) s))))))
         (proc-data (citre-make-async-process cmd callback))
         (proc (citre-process-proc proc-data)))
    (unwind-protect
        (progn
          (while (null finish-status) (accept-process-output))
          (pcase finish-status
            ('success (while (accept-process-output proc)))
            ('fail (error err-msg))
            (_ (error "Invalid FINISH-STATUS")))
          result)
      (citre-destruct-process proc-data))))
heartnheart commented 10 months ago

I couldn't reproduce it today after reverting back to:

(while (not finished) (accept-process-output))

I'll let you know if it happens again.

heartnheart commented 10 months ago

Hi @AmaiKinono, the issue happens again today but the new version for citre-get-output-lines does not fix it.

The (while (eq (process-status proc) 'run) (accept-process-output)) method works.

AmaiKinono commented 6 months ago

@heartnheart polling for process-status works for me and you, but causes our CI to fail.

I've come up with a new fix, which works for me and the CI. Could you try it?

If you are interested in the whole story, see the comment on the develop branch.

(defun citre-get-output-lines (cmd)
  (let* ((result nil)
         (err-msg nil)
         (finish-status nil)
         (callback
          (lambda (status msg)
            (pcase status
              ('output (setq result
                             (nconc result (split-string msg "\n" t))))
              (0 (setq finish-status 'success))
              ((and s (pred integerp))
               (setq finish-status 'fail
                     err-msg (format "Process %s exits %s:\n%s"
                                     (car cmd) s msg)))
              ('signal (setq finish-status 'fail))
              (s (setq finish-status 'fail
                       err-msg (format "Abnormal status of process %s:\n%s"
                                       (car cmd) s))))))
         (proc-data (citre-make-async-process cmd callback))
         (proc (citre-process-proc proc-data)))
    (unwind-protect
        (progn
          (while (eq (process-status proc) 'run) (accept-process-output))
          (while (null finish-status) (accept-process-output proc nil 50))
          (pcase finish-status
            ('success (accept-process-output proc))
            ('fail (error err-msg))
            (_ (error "Invalid FINISH-STATUS %s" finish-status)))
          result)
      (citre-destruct-process proc-data))))
AmaiKinono commented 3 months ago

News on this problem:

We do get several report of the freezing accept-process-output call, but none of Citre developers can reproduce or analyze it, so I'm closing this issue. Feel free to reopen if you have further clue or solution.