[bug#41658] [PATCH] fixes / improvements for (guix store database)

[Danny Milosavljevic]

Hi Ludo,
Hi Caleb,

On Thu, 04 Jun 2020 18:40:35 +0200
Ludovic Courtès <ludo@gnu.org> wrote:

> Nice.  It would be great if you could report it upstream (Danny and/or
> myself can then patch it directly in guile-sqlite3 and push out a
> release) and refer to the issue from here.

I agree.  It's easy to change sqlite-finalize in guile-sqlite3 to
call sqlite-reset, basically just adapt

(define sqlite-finalize
  (let ((f (pointer->procedure
            int
            (dynamic-func "sqlite3_finalize" libsqlite3)
            (list '*))))
    (lambda (stmt)
      ;; Note: When STMT is cached, this is a no-op.  This ensures caching
      ;; actually works while still separating concerns: users can turn
      ;; caching on and off without having to change the rest of their code.
      (when (and (stmt-live? stmt)
                 (not (stmt-cached? stmt)))
        (let ((p (stmt-pointer stmt)))
          (sqlite-remove-statement! (stmt->db stmt) stmt)
          (set-stmt-live?! stmt #f)
          (f p))))))

so that it calls sqlite-reset in the "when"'s new "else" branch there.

(we could also always call sqlite3_reset on sqlite-finalize anyway, it wouldn't
hurt but it wouldn't help either)

I agree that sqlite-finalize should model sqlite's finalization behavior as
much as possible.

Also, the comment about this being a no-op is not true then anymore.

We should definitely also pick up Caleb's comment upstream:

+  ;; Cached statements aren't reset when sqlite-finalize is invoked on
+  ;; them. This can cause problems with automatically-started transactions:
+  ;;
+  ;; "An implicit transaction (a transaction that is started automatically,
+  ;; not a transaction started by BEGIN) is committed automatically when the
+  ;; last active statement finishes. A statement finishes when its last cursor
+  ;; closes, which is guaranteed to happen when the prepared statement is
+  ;; reset or finalized. Some statements might "finish" for the purpose of
+  ;; transaction control prior to being reset or finalized, but there is no
+  ;; guarantee of this."
+  ;;
+  ;; Thus, it's possible for an implicitly-started transaction to hang around
+  ;; until sqlite-reset is called when the cached statement is next
+  ;; used. Because the transaction is committed automatically only when the
+  ;; *last active statement* finishes, the implicitly-started transaction may
+  ;; later be upgraded to a write transaction (!) and this non-reset statement
+  ;; will still be keeping the transaction from committing until it is next
+  ;; used or the database connection is closed. This has the potential to make
+  ;; (exclusive) write access to the database necessary for much longer than
+  ;; it should be.
+  ;;
+  ;; (see https://www.sqlite.org/lang_transaction.html)

@Caleb:

Could you file an issue at 
https://notabug.org/guile-sqlite3/guile-sqlite3/issues
and pull request so this is auditable?

pgpUiMkHXgAnP.pgp
Description: OpenPGP digital signature