bug#36556: 26.2; python.el pdbtracking sometimes kills buffers when it shouldn't (plus fix)

bug#36556: 26.2; python.el pdbtracking sometimes kills buffers when it shouldn't (plus fix)

[Ken Manheimer]

[-- Attachment #1: Type: text/plain, Size: 10335 bytes --]

python.el has a feature whereby the editor will progressively present the
python code lines that the user is stepping through in a pdb session in a
python buffer (or any comint buffer to which the user has added the filter
functions). The python.el pdb tracking provisions includes a function,
`python-pdbtrack-set-tracked-buffer()`, which registers python code buffers
that were created to visit files for pdbtracking for subsequent deletion
(the buffers, not the files) after the pdb session concludes. The problem
is that this provision sometimes registers buffers that were present in the
editing session before the pdbtracking session, so that pdbtracking
sometimes causes pdbtracked buffers to be deleted when it shouldn't.

I'm unsure what exact conditions lead to the problem, but I'm pretty sure
that `python-pdbtrack-set-tracked-buffer()` uses the wrong buffer-finding
function. Instead of using `get-file-buffer()`, it should be using
`find-buffer-visiting()`. I believe that this will solve the problem.

Here's a diff of python.el from the emacs-26.2.90 branch with my suggested
change applied. (The same patch would obtain for the master branch):

diff --git a/lisp/progmodes/python.el b/lisp/progmodes/python.el
index 38dcc823d0..151b27aa80 100644
--- a/lisp/progmodes/python.el
+++ b/lisp/progmodes/python.el
@@ -3717,7 +3717,7 @@ python-pdbtrack-set-tracked-buffer
 Returns the tracked buffer."
   (let* ((file-name-prospect (concat (file-remote-p default-directory)
                               file-name))
-         (file-buffer (get-file-buffer file-name-prospect)))
+         (file-buffer (find-buffer-visiting file-name-prospect)))
     (if file-buffer
         (setq python-pdbtrack-tracked-buffer file-buffer)
       (cond

I have verified that pdbtracking will delete some buffers that were created
just for tracking and keep some buffers that were present before tracking,
but haven't used it long enough to be sure it will do the right thing with
all buffers. But I'm pretty sure that find-buffer-visiting should be used
instead of get-file-buffer, so am pretty confident that this is a proper
change.

Ken Manheimer
ken.manheimer@gmail.com



In GNU Emacs 26.2 (build 1, x86_64-apple-darwin18.2.0, NS appkit-1671.20
Version 10.14.3 (Build 18D109))  of 2019-04-12 built on
builder10-14.porkrind.org Windowing system distributor 'Apple', version
10.3.1671
Recent messages:
Checking 87 files in /Applications/Emacs.app/Contents/Resources/lisp/calc...
Checking 105 files in
/Applications/Emacs.app/Contents/Resources/lisp/obsolete...
Checking for load-path shadows...done
Saved text from "



In GNU Emacs 26.2 (build 1, x86_64-a"
mwheel-scroll: Beginning of buffer [11 times]
previous-line: Beginning of buffer
Quit [5 times]
Configured using:
 'configure --with-ns '--enable-locallisppath=/Library/Application
 Support/Emacs/${version}/site-lisp:/Library/Application
 Support/Emacs/site-lisp' --with-modules'

Configured features:
NOTIFY ACL GNUTLS LIBXML2 ZLIB TOOLKIT_SCROLL_BARS NS MODULES THREADS

Important settings:
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Python

Minor modes in effect:
  flycheck-mode: t
  yas-minor-mode: t
  highlight-indentation-mode: t
  company-mode: t
  elpy-mode: t
  pyvenv-mode: t
  flymake-mode: t
  auto-revert-mode: t
  diff-hl-flydiff-mode: t
  save-place-mode: t
  show-paren-mode: t
  recentf-mode: t
  rcirc-track-minor-mode: t
  global-whitespace-mode: t
  global-diff-hl-mode: t
  diff-hl-mode: t
  diff-auto-refine-mode: t
  desktop-save-mode: t
  rainbow-delimiters-mode: t
  fci-mode: t
  savehist-mode: t
  shell-dirtrack-mode: t
  global-undo-tree-mode: t
  undo-tree-mode: t
  icomplete-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

Load-path shadows:
/Users/klm/lib/emacs/EmacsMultishell/multishell hides
/Users/klm/lib/emacs/EmacsUtils/multishell
/Users/klm/lib/emacs/misc hides
/Applications/Emacs.app/Contents/Resources/lisp/misc
/Users/klm/lib/emacs/align hides
/Applications/Emacs.app/Contents/Resources/lisp/align
/Users/klm/lib/emacs/allout_dev/allout hides
/Applications/Emacs.app/Contents/Resources/lisp/allout
/Users/klm/lib/emacs/allout_dev/allout-widgets hides
/Applications/Emacs.app/Contents/Resources/lisp/allout-widgets
/Users/klm/lib/emacs/css-mode hides
/Applications/Emacs.app/Contents/Resources/lisp/textmodes/css-mode
/Users/klm/lib/emacs/rst hides
/Applications/Emacs.app/Contents/Resources/lisp/textmodes/rst
/Users/klm/lib/emacs/nxml/rng-uri hides
/Applications/Emacs.app/Contents/Resources/lisp/nxml/rng-uri
/Users/klm/lib/emacs/nxml/rng-pttrn hides
/Applications/Emacs.app/Contents/Resources/lisp/nxml/rng-pttrn
/Users/klm/lib/emacs/nxml/rng-match hides
/Applications/Emacs.app/Contents/Resources/lisp/nxml/rng-match
/Users/klm/lib/emacs/nxml/nxml-maint hides
/Applications/Emacs.app/Contents/Resources/lisp/nxml/nxml-maint
/Users/klm/lib/emacs/nxml/rng-dt hides
/Applications/Emacs.app/Contents/Resources/lisp/nxml/rng-dt
/Users/klm/lib/emacs/nxml/xmltok hides
/Applications/Emacs.app/Contents/Resources/lisp/nxml/xmltok
/Users/klm/lib/emacs/nxml/nxml-enc hides
/Applications/Emacs.app/Contents/Resources/lisp/nxml/nxml-enc
/Users/klm/lib/emacs/nxml/rng-util hides
/Applications/Emacs.app/Contents/Resources/lisp/nxml/rng-util
/Users/klm/lib/emacs/nxml/rng-loc hides
/Applications/Emacs.app/Contents/Resources/lisp/nxml/rng-loc
/Users/klm/lib/emacs/nxml/rng-parse hides
/Applications/Emacs.app/Contents/Resources/lisp/nxml/rng-parse
/Users/klm/lib/emacs/nxml/xsd-regexp hides
/Applications/Emacs.app/Contents/Resources/lisp/nxml/xsd-regexp
/Users/klm/lib/emacs/nxml/rng-xsd hides
/Applications/Emacs.app/Contents/Resources/lisp/nxml/rng-xsd
/Users/klm/lib/emacs/nxml/rng-cmpct hides
/Applications/Emacs.app/Contents/Resources/lisp/nxml/rng-cmpct
/Users/klm/lib/emacs/nxml/nxml-mode hides
/Applications/Emacs.app/Contents/Resources/lisp/nxml/nxml-mode
/Users/klm/lib/emacs/nxml/rng-valid hides
/Applications/Emacs.app/Contents/Resources/lisp/nxml/rng-valid
/Users/klm/lib/emacs/nxml/nxml-ns hides
/Applications/Emacs.app/Contents/Resources/lisp/nxml/nxml-ns
/Users/klm/lib/emacs/nxml/nxml-util hides
/Applications/Emacs.app/Contents/Resources/lisp/nxml/nxml-util
/Users/klm/lib/emacs/nxml/rng-maint hides
/Applications/Emacs.app/Contents/Resources/lisp/nxml/rng-maint
/Users/klm/lib/emacs/nxml/nxml-parse hides
/Applications/Emacs.app/Contents/Resources/lisp/nxml/nxml-parse
/Users/klm/lib/emacs/nxml/rng-nxml hides
/Applications/Emacs.app/Contents/Resources/lisp/nxml/rng-nxml

Features:
(pp shadow sort mail-extr gnus-msg gnus-art mm-uu mml2015 mm-view
mml-smime smime dig mailcap gnus-sum gnus-group gnus-undo gnus-start
gnus-cloud nnimap nnmail mail-source tls gnutls utf7 netrc nnoo
gnus-spec gnus-int gnus-range gnus-win emacsbug message rmc puny rfc822
mml mml-sec epa mm-decode mm-bodies mm-encode mail-parse rfc2231
mailabbrev gmm-utils mailheader sendmail edebug compare-w add-log
log-view pcvs-util view sh-script smie executable time-stamp misearch
multi-isearch conf-mode markdown-mode noutline outline flycheck rx dash
company-oddmuse company-keywords company-etags company-gtags
company-dabbrev-code company-dabbrev company-files company-cmake
company-xcode company-clang company-eclim company-template company-bbdb
cl-extra yasnippet help-mode highlight-indentation company-capf company
elpy find-file-in-project ivy colir color ivy-overlay ffap windmove
elpy-shell pyvenv esh-var esh-io esh-cmd esh-opt esh-ext esh-proc
esh-arg esh-groups eshell esh-module esh-mode esh-util elpy-profile
elpy-django s elpy-refactor subr-x ido files-x etags xref project
cus-edit flymake-proc flymake warnings thingatpt python json map
autorevert filenotify diff-hl-flydiff disp-table elec-pair saveplace
jka-compr paren recentf tree-widget rcirc-reconnect rcirc-notify rcirc
avoid gnus nnheader gnus-util rmail rmail-loaddefs rfc2047 rfc2045
ietf-drums mail-utils mm-util mail-prsvr whitespace hl-line diff-hl
face-remap vc-hg vc-git vc-dir ewoc diff-mode easy-mmode desktop
frameset allout-widgets wid-edit allout cus-start cus-load delsel
mouse-drag flyspell ispell server monkey monkey-dir background nice
fields modal dired dired-loaddefs wgrep grep compile rainbow-delimiters
fill-column-indicator multishell multishell-list derived savehist pcase
user-info ange-ftp tramp-ftp tramp-sh tramp tramp-compat tramp-loaddefs
trampver shell pcomplete comint ansi-color ring parse-time format-spec
advice file-registry namedmarks vc vc-dispatcher misc undo-tree diff
icomplete-fix nxml-mode nxml-util nxml-glyph nxml-enc xmltok find-func
retain-copy cl epg icomplete edmacro kmacro finder-inf info package
easymenu epg-config url-handlers url-parse auth-source cl-seq eieio
eieio-core cl-macs eieio-loaddefs password-cache url-vars seq byte-opt
gv bytecomp byte-compile cconv cl-loaddefs cl-lib time-date tooltip
eldoc electric uniquify ediff-hook vc-hooks lisp-float-type mwheel
term/ns-win ns-win ucs-normalize mule-util term/common-win tool-bar dnd
fontset image regexp-opt fringe tabulated-list replace newcomment
text-mode elisp-mode lisp-mode prog-mode register page menu-bar
rfn-eshadow isearch timer select scroll-bar mouse jit-lock font-lock
syntax facemenu font-core term/tty-colors frame cl-generic cham georgian
utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao korean
japanese eucjp-ms cp51932 hebrew greek romanian slovak czech european
ethiopic indian cyrillic chinese composite charscript charprop
case-table epa-hook jka-cmpr-hook help simple abbrev obarray minibuffer
cl-preloaded nadvice loaddefs button faces cus-face macroexp files
text-properties overlay sha1 md5 base64 format env code-pages mule
custom widget hashtable-print-readable backquote threads kqueue cocoa ns
multi-tty make-network-process emacs)

Memory information:
((conses 16 701576 48230)
 (symbols 48 47329 11)
 (miscs 40 17455 3460)
 (strings 32 120830 4193)
 (string-bytes 1 3766078)
 (vectors 16 80685)
 (vector-slots 8 2053353 146650)
 (floats 8 517 2021)
 (intervals 56 29302 12)
 (buffers 992 154))

[-- Attachment #2: Type: text/html, Size: 13303 bytes --]

bug#36556: 26.2; python.el pdbtracking sometimes kills buffers when it shouldn't (plus fix)

[Noam Postavsky]

Ken Manheimer <ken.manheimer@gmail.com> writes:

> The problem is that this provision sometimes registers buffers that
> were present in the editing session before the pdbtracking session, so
> that pdbtracking sometimes causes pdbtracked buffers to be deleted
> when it shouldn't.
>
> I'm unsure what exact conditions lead to the problem,

I guess this can happen if your python source files are accessible
through symlinks?

> but I'm pretty sure
> that `python-pdbtrack-set-tracked-buffer()` uses the wrong buffer-finding
> function. Instead of using `get-file-buffer()`, it should be using
> `find-buffer-visiting()`. I believe that this will solve the problem.

I think this should be conditional on (or find-file-existing-other-name
find-file-visit-truename), which is what find-file-noselect checks for.

bug#36556: 26.2; python.el pdbtracking sometimes kills buffers when it shouldn't (plus fix)

[Ken Manheimer]

[-- Attachment #1: Type: text/plain, Size: 1568 bytes --]

On Wed, Jul 17, 2019 at 11:03 AM Noam Postavsky <npostavs@gmail.com> wrote:

> Ken Manheimer <ken.manheimer@gmail.com> writes:
>
> > The problem is that this provision sometimes registers buffers that
> > were present in the editing session before the pdbtracking session, so
> > that pdbtracking sometimes causes pdbtracked buffers to be deleted
> > when it shouldn't.
> >
> > I'm unsure what exact conditions lead to the problem,
>
> I guess this can happen if your python source files are accessible
> through symlinks?
>

Yes, that turned out to be the situation in the cases I encountered. There
are probably other ways a file can be found via multiple paths, like
hardlinks and multiple mounts of a filesystem.

> but I'm pretty sure
> > that `python-pdbtrack-set-tracked-buffer()` uses the wrong buffer-finding
> > function. Instead of using `get-file-buffer()`, it should be using
> > `find-buffer-visiting()`. I believe that this will solve the problem.
>
> I think this should be conditional on (or find-file-existing-other-name
> find-file-visit-truename), which is what find-file-noselect checks for.
>

No, as far as I can tell, that doesn't hold in this situation.

The point of this issue is to not delete a source file that was already
present in the editing session before the pdb-tracking debugging process
stepped through code in the file. The prior presence of the file should be
respected regardless of the settings of find-file-existing-other-name or
find-file-visit-truename, so these settings should not be considered for
this comparison.

Ken

[-- Attachment #2: Type: text/html, Size: 2380 bytes --]

bug#36556: 26.2; python.el pdbtracking sometimes kills buffers when it shouldn't (plus fix)

[Noam Postavsky]

Ken Manheimer <ken.manheimer@gmail.com> writes:

>> > that `python-pdbtrack-set-tracked-buffer()` uses the wrong buffer-finding
>> > function. Instead of using `get-file-buffer()`, it should be using
>> > `find-buffer-visiting()`. I believe that this will solve the problem.
>>
>> I think this should be conditional on (or find-file-existing-other-name
>> find-file-visit-truename), which is what find-file-noselect checks for.
>>
>
> No, as far as I can tell, that doesn't hold in this situation.
>
> The point of this issue is to not delete a source file that was already
> present in the editing session before the pdb-tracking debugging process
> stepped through code in the file. The prior presence of the file should be
> respected regardless of the settings of find-file-existing-other-name or
> find-file-visit-truename, so these settings should not be considered for
> this comparison.

My understanding is that python-pdbtrack-set-tracked-buffer has to
decide whether or not to (a) make a new buffer for visiting FILE-NAME
(in which case pdb should kill that buffer when done with it), or (b)
use an existing one (in which case, the buffer should be left open).

If (or find-file-visit-truename find-file-visit-truename) is t, and
FILE-NAME is visitied via an alternate name, then the current
implementation will get nil from get-file-buffer, create a "new" buffer
with find-file-noselect (and register that buffer in
python-pdbtrack-buffers-to-kill).  However, find-file-noselect will
return an existing buffer, so killing this buffer is a bug (i.e., the
function think it's in case (a), but it's actually in case (b)).

If (or find-file-visit-truename find-file-visit-truename) is nil, then
find-file-noselect would really return a new buffer in that situation,
so it would be correct to kill it (i.e., with these settings, we are in
case (a)).

Do I have a mistake in the above?

bug#36556: 26.2; python.el pdbtracking sometimes kills buffers when it shouldn't (plus fix)

[Lars Ingebrigtsen]

Noam Postavsky <npostavs@gmail.com> writes:

> If (or find-file-visit-truename find-file-visit-truename) is nil, then
> find-file-noselect would really return a new buffer in that situation,
> so it would be correct to kill it (i.e., with these settings, we are in
> case (a)).
>
> Do I have a mistake in the above?

This was over a year ago, and there was no response to this question, so
I'm closing this bug report.  If the problem still exists, please
respond to this email and we'll reopen the bug report.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#36556: 26.2; python.el pdbtracking sometimes kills buffers when it shouldn't (plus fix)

[Ken Manheimer]

[-- Attachment #1: Type: text/plain, Size: 878 bytes --]

I'm sorry I didn't reply. I think Noam's analysis makes sense, and what he
suggests should be implemented. The problem is real, and should be
rectified.

Ken

On Sun, Aug 9, 2020 at 3:32 PM Lars Ingebrigtsen <larsi@gnus.org> wrote:

> Noam Postavsky <npostavs@gmail.com> writes:
>
> > If (or find-file-visit-truename find-file-visit-truename) is nil, then
> > find-file-noselect would really return a new buffer in that situation,
> > so it would be correct to kill it (i.e., with these settings, we are in
> > case (a)).
> >
> > Do I have a mistake in the above?
>
> This was over a year ago, and there was no response to this question, so
> I'm closing this bug report.  If the problem still exists, please
> respond to this email and we'll reopen the bug report.
>
> --
> (domestic pets only, the antidote for overdose, milk.)
>    bloggy blog: http://lars.ingebrigtsen.no
>

[-- Attachment #2: Type: text/html, Size: 1404 bytes --]

bug#36556: 26.2; python.el pdbtracking sometimes kills buffers when it shouldn't (plus fix)

[Lars Ingebrigtsen]

Ken Manheimer <ken.manheimer@gmail.com> writes:

> I'm sorry I didn't reply. I think Noam's analysis makes sense, and
> what he suggests should be implemented. The problem is real, and
> should be rectified.

OK; reopening.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no