bug#25656: 25.1; remember-notes-mode makes every temporary buffer created from it unkillable

bug#25656: 25.1; remember-notes-mode makes every temporary buffer created from it unkillable

[Vasilij Schneidermann]

I wrote an Emacs package that takes an Org file, processes it with Org's
element API and exports to a file.  Later I've noticed that if I use it
on an Org buffer created with remember-notes-mode, it creates ~500
temporary buffers that cannot be killed.  Bisection reveals that this is
the fault of `remember-notes--kill-buffer-query` which is added locally
to `kill-buffer-query-functions` and returns nil if
`remember-notes-bury-on-kill` hasn't been customized.

This raises an interesting question, should a temporary buffer as
created by `with-temp-buffer` let-bind potentially troublesome hooks
including, but not limited to `kill-buffer-query-functions`?  Another
example coming to mind is `buffer-list-update-hook`, adding a function
to it that scans the buffer list makes creating and killing temporary
buffers an expensive action.

bug#25656: [PATCH] 25.1; remember-notes-mode makes every temporary buffer created from it unkillable

[Vasilij Schneidermann]

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

I wrote a simple patch for merely checking whether the buffer about to
be killed happens to be the remember-notes buffer.

[-- Attachment #2: 0001-Make-only-remember-notes-buffer-unkillable.patch --]
[-- Type: text/x-diff, Size: 1070 bytes --]

From b1f95cf53fdb3574525f3644df668f357405f4a1 Mon Sep 17 00:00:00 2001
From: Vasilij Schneidermann <v.schneidermann@gmail.com>
Date: Fri, 24 Feb 2017 08:36:38 +0100
Subject: [PATCH] Make only remember-notes buffer unkillable

* remember.el (remember-notes--kill-buffer-query): Check whether
  buffer to be killed is the remember-notes buffer
---
 lisp/textmodes/remember.el | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lisp/textmodes/remember.el b/lisp/textmodes/remember.el
index 388e49cfdc..a217357471 100644
--- a/lisp/textmodes/remember.el
+++ b/lisp/textmodes/remember.el
@@ -649,7 +649,8 @@ Save the current buffer if modified.  If `remember-notes-bury-on-kill'
 is non-nil, bury it and return nil; otherwise return t."
   (when (buffer-modified-p)
     (save-buffer))
-  (if remember-notes-bury-on-kill
+  (if (and remember-notes-bury-on-kill
+           (equal (buffer-name (current-buffer)) remember-notes-buffer-name))
       (progn
         ;; bury-buffer always returns nil, but let's be explicit.
         (bury-buffer)
-- 
2.11.0

bug#25656: [PATCH] 25.1; remember-notes-mode makes every temporary buffer created from it unkillable

[npostavs]

Vasilij Schneidermann <v.schneidermann@gmail.com> writes:

> I wrote a simple patch for merely checking whether the buffer about to
> be killed happens to be the remember-notes buffer.

I think that's a sensible change, but doesn't the fact that
remember-mode adds the hook buffer-locally mean that temp buffers (or
any other buffers) aren't affected?

    (add-hook 'kill-buffer-query-functions 'XXXX nil t)
    (with-temp-buffer
      kill-buffer-query-functions) ;=> (process-kill-buffer-query-function)

bug#25656: [PATCH] 25.1; remember-notes-mode makes every temporary buffer created from it unkillable

[Vasilij Schneidermann]

> I think that's a sensible change, but doesn't the fact that
> remember-mode adds the hook buffer-locally mean that temp buffers (or
> any other buffers) aren't affected?

Hm, you're right, I can't repro this with a minimal example.  However
I'm sure that it has to do something with Org copying buffers and
failing to kill them, so I'll investigate a bit harder to see what it
could be, outside of its gratuitious use of `with-temp-buffer`.

bug#25656: [PATCH] 25.1; remember-notes-mode makes every temporary buffer created from it unkillable

[Vasilij Schneidermann]

OK, I think I've figured out what's going on.  I'm calling a command in
a remember-notes buffer that happens to hold an org file.  It's set up
to be unkillable by having a locally added `kill-buffer-query-functions`
hook that returns nil.  Now, my command calls a number of things
including `org-table-export` which does a zillion of things, eventually
cloning the buffer it's working on with the
`org-export-with-buffer-copy` macro.  This macro is so good at cloning
buffers it preserves its buffer-local variables, including
`kill-buffer-query-functions`.  Although it tries killing the buffer
afterwards, it fails doing so because the hook forbids it.

The result of this avalanche of failures is that for every table cell,
an unkillable buffer is created.  The lesson from this is to be careful
with using Org's APIs, while some of them look reusable, most aren't.
While I could patch `remember-notes--kill-buffer-query` to work around
this problem, I'll just customize `remember-notes-bury-on-kill` and
raise an issue with Org's bug tracker.  Thanks for responding to this,
Noam!

bug#25656: [PATCH] 25.1; remember-notes-mode makes every temporary buffer created from it unkillable

[Vasilij Schneidermann]

Bug reason found in Org, resubmitting issue on their bug tracker.