Skip to content
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.

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

Fix inconsistent tx state with database/sql. #300

Merged
merged 1 commit into from
Jul 15, 2016
Merged

Conversation

sqweek
Copy link
Contributor

@sqweek sqweek commented Apr 18, 2016

The semantics of sql.Tx.Commit impose that the transaction is
finished and cleaned up by the time the driver's Commit function
returns. However sqlite3 leaves the transaction open if COMMIT
fails due to an SQLITE_BUSY error, so we must clean it up.

Closes #184.

The semantics of sql.Tx.Commit impose that the transaction is
finished and cleaned up by the time the driver's Commit function
returns. However sqlite3 leaves the transaction open if COMMIT
fails due to an SQLITE_BUSY error, so *we* must clean it up.

Closes mattn#184.
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.4%) to 66.38% when pulling 727ad20 on sqweek:issue184 into 22d7351 on mattn:master.

@sqweek
Copy link
Contributor Author

sqweek commented Apr 18, 2016

To test the change I used this little program: https://play.golang.org/p/wnMn2OVhfn

  1. Run the program a few times to create the database (/tmp/issue184.db) and insert some rows
  2. In a separate terminal run the commands:
    1. sqlite3 /tmp/issue184.db
    2. sqlite> BEGIN;
    3. sqlite> SELECT * FROM tab;
  3. The sqlite3 process has a read-lock on the database. Run the program again.
  4. The program will get a database is locked error trying to COMMIT, as expected.
  5. The program sleeps for 5 seconds and tries to tx.Commit() again, which fails because database/sql considers the transaction finished
  6. The program tries to tx.Rollback() which fails for the same reason
  7. The program calls db.Begin() to open a new transaction, which succeeds

Before the change, the db.Begin() call failed with error cannot start a transaction within a transaction.

I'm not sure how easy it would be to isolate this into a test case.

@mattn mattn merged commit e118d44 into mattn:master Jul 15, 2016
@mattn
Copy link
Owner

mattn commented Jul 15, 2016

Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants