Subject: Re: [bug#69292] [PATCH 2/6] store: database: Remove with-statement and associated code.

From: Reepca Russelstein <[email-masked]>
Date: Fri, 23 Feb 2024 12:32:03 -0600

>> I think using dynamic-wind to finalize all statements is the wrong
>> approach. Firstly it would be good to allow reseting statements rather than
>> finalizing them.

Let me once again mention https://issues.guix.gnu.org/69292#7 with this
much more magnificent Cc list.

>> Then for the problem of handling errors, the approach I've
>> settled on in the build coordinator is to close the database connection, since
>> that'll trigger guile-sqlite3 to finalize all the cached statements.

And in the event that a statement is *not* cached, it will hang around
until the gc next pumps the statement guardian, at which point it will
do... whatever happens when a statement is finalized after the database
connection it was created with has already been closed, I guess.  I
don't know if that's a problem or not.  On further investigation, it
appears that sqlite_close would return SQLITE_BUSY, but guile-sqlite3's
sqlite-close doesn't throw any exceptions, and according to
https://www.sqlite.org/c3ref/close.html it would just hold off on
actually closing the database until all statements have been finalized.
So I guess that works.

>>
>> This reverts commit 5d6e2255286e591def122ec2f4a3cbda497fea21.
>>
>> * .dir-locals.el (scheme-mode): Remove with-statement.
>> * guix/store/database.scm (call-with-statement): Remove procedure.
>> (with-statement): Remove syntax rule.
>> (call-with-transaction, last-insert-row-id, path-id, update-or-insert,
>> add-references): Don't use with-statement.
>>
>> Change-Id: I2fd976b3f12ec8105cc56350933a953cf53647e8
>
> I’m all for removing ‘dynamic-wind’, we’ll have to do it to make it
> usable in a fiberized context anyway.
>
> I’ll let reepca comment.

What is the proper fibers-friendly replacement for dynamic-wind, anyway,
that is "like dynamic-wind, except when suspending a fiber"?  It feels
like the current interaction between dynamic-wind and fibers is more of
an accident of how the implementation works; I don't suppose fibers could
export a transparent replacement, like how it already exports a
replacement for 'sleep'?  Or perhaps the underlying issue is that we
keep using 'dynamic-wind' in situations where it only makes sense to
enter or exit the dynamic extent once?  Is it time to bring
'unwind-protect' back into style?  I see that fibers now has a
dynamic-wind*, should that be preferred?

I don't have a strong opinion on these changes, I just want to make sure
we're all aware of how guile-sqlite3's sqlite-finalize acts with cached
statements.

- reepca