sqliteutil.Save does not rollback properly after SQLITE_INTERRUPT about sqlite HOT 6 CLOSED

Turns out an IsInterrupted method would have been racy. So I made SetInterrupt return the old doneCh so the releaseFn can swap it out and back. Thanks!

from sqlite.

You're absolutely right that ROLLBACK should run after interrupt. Tricky! This probably needs more API surface in the sqlite package. Let me see what I can come up with.

from sqlite.

You're absolutely right that ROLLBACK should run after interrupt. Tricky! This probably needs more API surface in the sqlite package. Let me see what I can come up with.

One idea is to

releaseFn := func {
  if needRollback {
    defer conn.BypassInterrupts()()
    Exec("... ROLLBACK ...")
  }
}

Where BypassInterrupts would look like:

func (conn *Conn) BypassInterrupts() func {
  conn.bypassInterrupts = true
  return conn.enableInterrupts
}

func (conn *Conn) enableInterrupts() {
  conn.bypassInterrupts = false
}

(bypassInterrupts would be checked in conn.interrupted)

I don't have a precise idea of how costly defer are, but I like this pattern a lot (discovered it in this library!), because it makes it pretty much impossible to accidentally end up with conn.bypassInterrupts = false

from sqlite.

I think an IsInterrupted method on conn is enough. Then must-run code like rollback can:

interrupted := conn.IsInterrupted()
if interrupted {
    conn.SetInterrupt(nil)
}
// execute ROLLBACK
if interrupted {
    ch := make(chan struct{})
    close(ch)
    conn.SetInterrupt(ch)
}

from sqlite.

LGTM - it sorta bothers me that doneCh's value will be changed, but I guess if we've been interrupted, it doesn't matter.

(Also maybe cache the closed chan in a package-level variable if make()-ing it isn't zero-cost and that doesn't have terrible consequences down the line?)

from sqlite.

This issue was opened under the faulty assumption that SAVEPOINTs always need to be rolled back and released after a SQLITE_INTERRUPT occurs. But there are times when the SAVEPOINTs are automatically rolled back.

This introduced a different subtle bug described in #73

from sqlite.