Skip to content

Guard against stale markers in async callbacks #281

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.

Sign up for GitHub

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

Open
benthamite wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Guard against stale markers in async callbacks #281

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fcc8cdf
Guard against stale markers in async callbacks
benthamite Mar 21, 2026
3277e77
Add stale marker regression tests
telotortium Apr 25, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 18 additions & 2 deletions org-gcal.el
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -743,6 +743,10 @@ have been moved from the default fetch file. CALENDAR-ID is defined in
(let ((marker (or (org-gcal--event-entry-marker entry)
(org-gcal--id-find (org-gcal--event-entry-entry-id entry))))
(event (org-gcal--event-entry-event entry)))
(when (and (markerp marker)
(not (marker-buffer marker)))
(error "org-gcal: marker's buffer for entry %s has been killed"
(org-gcal--event-entry-entry-id entry)))
(org-with-point-at marker
;; If skipping exports, just overwrite current entry's
;; calendar data with what's been retrieved from the
Expand Down Expand Up @@ -819,12 +823,16 @@ to “org”."
(defmacro org-gcal--with-point-at-no-widen (pom &rest body)
"Move to buffer and point of point-or-marker POM for the duration of BODY.

Based on ‘org-with-point-at’ but doesn’t widen the buffer."
Based on ‘org-with-point-at’ but doesn’t widen the buffer.
Signals an error if POM is a marker whose buffer has been killed."
(declare (debug (form body)) (indent 1))
(org-with-gensyms (mpom)
`(let ((,mpom ,pom))
(save-excursion
(when (markerp ,mpom) (set-buffer (marker-buffer ,mpom)))
(when (markerp ,mpom)
(unless (marker-buffer ,mpom)
(error "org-gcal: marker’s buffer has been killed"))
(set-buffer (marker-buffer ,mpom)))
(goto-char (or ,mpom (point)))
,@body))))

Expand Down Expand Up @@ -1354,6 +1362,8 @@ delete calendar info from events on calendars you no longer have access to."
(message "clear-gcal-info delete-error: %S %S"
clear-gcal-info delete-error)
(when (or clear-gcal-info (null delete-error))
(unless (marker-buffer marker)
(error "org-gcal: marker’s buffer has been killed"))
;; Delete :org-gcal: drawer after deleting event. This will preserve
;; the ID for links, but will ensure functions in this module don’t
;; identify the entry as a Calendar event.
Expand Down Expand Up @@ -1976,6 +1986,8 @@ Returns a ‘deferred’ object that can be used to wait for completion."
(org-gcal--get-event calendar-id event-id)
(deferred:nextc it
(lambda (response)
(unless (marker-buffer marker)
(error "org-gcal: marker's buffer has been killed"))
(save-excursion
(with-current-buffer (marker-buffer marker)
(goto-char (marker-position marker))
Expand All @@ -1995,6 +2007,8 @@ Returns a ‘deferred’ object that can be used to wait for completion."
(t
(unless skip-export
(let* ((data (request-response-data response)))
(unless (marker-buffer marker)
(error "org-gcal: marker's buffer has been killed"))
(save-excursion
(with-current-buffer (marker-buffer marker)
(goto-char (marker-position marker))
Expand Down Expand Up @@ -2080,6 +2094,8 @@ Returns a ‘deferred’ object that can be used to wait for completion."
(org-gcal--get-event calendar-id event-id)
(deferred:nextc it
(lambda (response)
(unless (marker-buffer marker)
(error "org-gcal: marker's buffer has been killed"))
(save-excursion
(with-current-buffer (marker-buffer marker)
(goto-char (marker-position marker))
Expand Down
130 changes: 130 additions & 0 deletions test/org-gcal-test.el
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,22 @@ always located at the beginning of the buffer."
(goto-char (point-min))
,@body))

(defun org-gcal-test--stale-marker (contents)
"Return a marker whose buffer was killed after inserting CONTENTS."
(let ((buffer (generate-new-buffer " *org-gcal-stale-marker*")))
(with-current-buffer buffer
(org-mode)
(insert contents)
(goto-char (point-min))
(prog1 (point-marker)
(kill-buffer buffer)))))

(defmacro org-gcal-test--should-error-match (regexp &rest body)
"Assert that BODY signals an error whose message matches REGEXP."
(declare (indent 1) (debug t))
`(let ((err (should-error (progn ,@body) :type 'error)))
(should (string-match-p ,regexp (error-message-string err)))))

(defun org-gcal-test--json-read-string (json)
"Wrap ‘org-gcal--json-read’ to parse a JSON string."
(with-temp-buffer
Expand Down Expand Up @@ -614,6 +630,41 @@ Second paragraph
(let ((org-gcal-managed-post-at-point-update-existing 'always-push))
(org-gcal-post-at-point)))))

(ert-deftest org-gcal-test--sync-update-entries-stale-marker ()
"Verify sync reports a clear error for entries with stale markers."
(let ((errors nil))
(cl-letf (((symbol-function 'deferred:loop)
(lambda (entries body)
(funcall body (car entries))))
((symbol-function 'error)
(lambda (format-string &rest args)
(let ((message (apply #'format format-string args)))
(push message errors)
(signal 'error (list message))))))
(ignore-errors
(deferred:sync!
(org-gcal--sync-update-entries
org-gcal-test-calendar-id
(list
(org-gcal--event-entry-create
:entry-id "foobar1234/foo@foobar.com"
:marker (org-gcal-test--stale-marker "* My event summary\n")
:event org-gcal-test-event))
t)))
(should
(cl-some
(lambda (error)
(string-match-p "marker.*buffer.*foobar1234/foo@foobar.com.*killed"
error))
errors)))))

(ert-deftest org-gcal-test--with-point-at-no-widen-stale-marker ()
"Verify stale markers report the killed buffer before moving point."
(org-gcal-test--should-error-match "marker.*buffer has been killed"
(org-gcal--with-point-at-no-widen
(org-gcal-test--stale-marker "* My event summary\n")
(point))))

(ert-deftest org-gcal-test--post-at-point-api-response ()
"Verify that ‘org-gcal-post-to-point’ updates an event using the data
returned from the Google Calendar API."
Expand Down Expand Up @@ -688,6 +739,27 @@ My event description
Second paragraph
"))))))))

(ert-deftest org-gcal-test--post-event-stale-marker ()
"Verify that post-event callbacks report stale markers clearly."
(let ((marker (org-gcal-test--stale-marker "* My event summary\n")))
(cl-letf (((symbol-function 'org-gcal--get-access-token)
(lambda (_calendar-id) "my_access_token"))
((symbol-function 'request-deferred)
(lambda (&rest _args)
(deferred:succeed
(make-request-response
:status-code 200
:data org-gcal-test-event)))))
(org-gcal-test--should-error-match "marker.*buffer has been killed"
(deferred:sync!
(org-gcal--post-event
"2019-10-06T17:00:00Z" "2019-10-06T21:00:00Z"
"My event summary" "Foobar's desk"
'((url . "https://google.com") (title . "Google"))
"My event description"
org-gcal-test-calendar-id
marker "opaque" "\"12344321\"" "foobar1234"))))))

(ert-deftest org-gcal-test--post-at-point-managed-update-existing-gcal ()
"Verify ‘org-gcal-post-at-point’ with ‘org-gcal-managed-update-existing-mode’
set to \"gcal\"."
Expand Down Expand Up @@ -1228,6 +1300,64 @@ Second paragraph
(deferred:sync! (org-gcal-delete-at-point))
(should (equal (buffer-string) "")))))))

(ert-deftest org-gcal-test--delete-event-stale-marker-on-etag-conflict ()
"Verify delete-event reports stale markers clearly after HTTP 412."
(let ((marker (org-gcal-test--stale-marker "* My event summary\n")))
(cl-letf (((symbol-function 'org-gcal--get-access-token)
(lambda (_calendar-id) "my_access_token"))
((symbol-function 'org-gcal--notify) #'ignore)
((symbol-function 'request-deferred)
(lambda (&rest _args)
(deferred:succeed
(make-request-response
:status-code 412))))
((symbol-function 'org-gcal--get-event)
(lambda (_calendar-id _event-id)
(deferred:succeed
(make-request-response
:status-code 200
:data org-gcal-test-event)))))
(org-gcal-test--should-error-match "marker.*buffer has been killed"
(deferred:sync!
(org-gcal--delete-event org-gcal-test-calendar-id
"foobar1234"
"\"12344321\""
marker))))))

(ert-deftest org-gcal-test--delete-at-point-stale-marker-in-finally ()
"Verify delete-at-point reports clearly if its source buffer is killed."
(org-gcal-test--with-temp-buffer
"\
* My event summary
:PROPERTIES:
:ETag: \"12344321\"
:calendar-id: foo@foobar.com
:entry-id: foobar1234/foo@foobar.com
:END:
:org-gcal:
My event description
:END:
"
(cl-letf (((symbol-function 'org-gcal--get-access-token)
(lambda (_calendar-id) "my_access_token"))
((symbol-function 'y-or-n-p)
(lambda (&rest _args) t))
((symbol-function 'org-gcal--delete-event)
(lambda (_calendar-id _event-id _etag marker &optional _a-token)
(kill-buffer (marker-buffer marker))
(deferred:succeed nil)))
((symbol-function 'error)
(lambda (format-string &rest args)
(let ((message (apply #'format format-string args)))
(when (string-match-p "marker.*buffer has been killed" message)
(throw 'stale-marker-error message))
(signal 'error (list message))))))
(should
(string-match-p "marker.*buffer has been killed"
(catch 'stale-marker-error
(deferred:sync! (org-gcal-delete-at-point))
nil))))))


(ert-deftest org-gcal-test--save-with-full-day-event ()
"Verify that a full day event will get set correctly."
Expand Down
Loading