Subject: bug#75209: 30.0.93; Emacs reader failed to read data in "/home/nlj/.cache/org-persist/gc-lock.eld"

From: "N. Jackson" <[email-masked]>
Date: Mon, 13 Jan 2025 15:36:04 +0000

At 16:34 +0200 on Saturday 2025-01-11, Eli Zaretskii wrote:
>
>> From: Ihor Radchenko <[email-masked]>
>> Cc: [email-masked], [email-masked]
>> Date: Sat, 11 Jan 2025 14:05:22 +0000
>> 
>> Ihor Radchenko <[email-masked]> writes:
>> 
>> Done on Org main.
>> https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=7999433067
>> https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=2a620113c1
>> 
>> I will not risk bugfix.
>> 
>> Hopefully, the bug is resolved now.
>
> Thanks, I hope the OP will be able to install the changes locally

I will do my best to do that.

The commits are against the Org "Main" branch whereas I'm running
Emacs 30.0.93 which I where I see the warnings, and the diffs don't
apply against this version of org-persist.el.

For example, in org-persist--write-elisp-file in Emacs 30.0.93 (at
line 481 or org-persist.el) -- in the thick of where these changes
are happening -- we have this horror:

    ;; Force writing even when the file happens to be opened by
    ;; another Emacs process.
    (cl-letf (((symbol-function #'ask-user-about-lock)
               ;; FIXME: Emacs 27 does not yet have `always'.
               (lambda (&rest _) t)))

This (thank goodness) had disappeared from the Org "Main" branch
before Ihor's two commits above.

IIUC Ihor's two commits make two independent fixes.

One fix (comprising all four changes in the first commit, and one of
the changes in the second commit) is to "Write index before writing
cache data.  This makes sure that index and data are always in
sync."

The other fix (comprising two of the three changes in the second
commit), is to ensure an atomic write by writing to a temp file and
then moving that into place.

I have insufficient knowledge of Elisp and no understanding, at all
really, of org-persist, so I am not able to attempt to make the
first fix here, and I'm not sure that it would make sense to even
try to do so.

The latter fix is simple and easy to think about so I have applied
those changes to my local version of the Emacs 30.0.93 org-persist
(patch below).  It seems quite plausible that this change (alone)
will fix the problem of the warnings in this bug report and the
broken timer.

> and verify them.

Verifying that any change resolves this bug is a bit tricky as it
involves waiting for nothing to happen!  That is, if the warning
reappears I'll be able to say that this change (alone) does not
(by itself) resolve the problem, but if the warning doesn't reappear
in a day, a week, a month, I won't be able to say that it never
will reappear.

Anyway, I will run with this change and I will wait and see what
happens.  If the problem is not solved, I wouldn't expect it to show
itself for several days, given its usual frequency.  I'll try to
put my machine into suspend for over an hour (so the timers fire
"simultaneously" on resume) as often as possible to hurry things
along.  I'll report back if the problem is not solved.

This is the change I have made here for testing:

--- Emacs-30.0.93/org-persist.el	2024-12-18 17:30:29.000000000 -0500
+++ atomic_write/org-persist.el	2025-01-12 18:58:58.230823402 -0500
@@ -475,7 +475,8 @@
         (print-escape-nonascii t)
         (print-continuous-numbering t)
         print-number-table
-        (start-time (float-time)))
+        (start-time (float-time))
+        (tmp-file (make-temp-file "org-persist-")))
     (unless (file-exists-p (file-name-directory file))
       (make-directory (file-name-directory file) t))
     ;; Force writing even when the file happens to be opened by
@@ -483,12 +484,19 @@
     (cl-letf (((symbol-function #'ask-user-about-lock)
                ;; FIXME: Emacs 27 does not yet have `always'.
                (lambda (&rest _) t)))
-      (with-temp-file file
+      ;; Do not write to FILE directly.  Another Emacs instance may be
+      ;; doing the same at the same time.  Instead, write to new
+      ;; temporary file and then rename it (renaming is atomic
+      ;; operation that does not create data races).
+      ;; See https://debbugs.gnu.org/cgi/bugreport.cgi?bug=75209#35
+      (with-temp-file tmp-file
         (insert ";;   -*- mode: lisp-data; -*-\n")
         (if pp
             (let ((pp-use-max-width nil)) ; Emacs bug#58687
               (pp data (current-buffer)))
-          (prin1 data (current-buffer)))))
+          (prin1 data (current-buffer))))
+      (rename-file tmp-file file 'overwrite))
+    
     (org-persist--display-time
      (- (float-time) start-time)
      "Writing to %S" file)))