pharo-rdbms / Pharo-SQLite3

Community-owned official SQLite3 binding for Pharo
MIT License
22 stars 20 forks source link

Finalizing statements causes Pharo 10.0 to crash #53

Closed gcorriga closed 1 year ago

gcorriga commented 2 years ago

Prepared statement finalization is causing Pharo 10 to crash on Windows. This can be reproduced using both the current SQLite3 version (30.38.5) and the one bundled with Pharo-SQLite3.

To reproduce, evaluate the following code in a Pharo 10 playground:

| connection statement |
connection := SQLite3Connection memory.
connection open.
connection execute: 'CREATE TABLE IF NOT EXISTS data(
        key TEXT PRIMARY KEY,
        value NOT NULL
        );'.
statement := connection prepare: 'INSERT INTO data VALUES(?1, ?2);'.
statement execute: #('Version' '20201125').
statement finalize

Image details: Pharo10.0.0 Build information: Pharo-10.0.0+build.520.sha.68bad00f7e8a4c4e1f720fe0ffbef8baa8ad607f (64 Bit)

VM Details: CoInterpreter VMMaker-tonel.1 uuid: 28fbbb79-d49c-0d00-b78c-10170af9ed93 Jun 2 2022 StackToRegisterMappingCogit VMMaker-tonel.1 uuid: 28fbbb79-d49c-0d00-b78c-10170af9ed93 Jun 2 2022 v9.0.15 - Commit: b4879008 - Date: 2022-06-02 09:48:57 +0200

Pharo 9.0.15 built on Jun 2 2022 10:27:30 Compiler: 4.2.1 Compatible Clang 8.0.1 (tags/RELEASE_801/final) VMMaker versionString v9.0.15 - Commit: b4879008 - Date: 2022-06-02 09:48:57 +0200 CoInterpreter VMMaker-tonel.1 uuid: 28fbbb79-d49c-0d00-b78c-10170af9ed93 Jun 2 2022 StackToRegisterMappingCogit VMMaker-tonel.1 uuid: 28fbbb79-d49c-0d00-b78c-10170af9ed93 Jun 2 2022

gcorriga commented 2 years ago

As per https://github.com/pharo-project/pharo/issues/11019 this seems linked to how parameters are bound and cleared.

However this snippet doesn't cause the crash

| connection statement |
connection := SQLite3Connection memory.
connection open.
connection execute: 'CREATE TABLE IF NOT EXISTS data(
        key TEXT PRIMARY KEY,
        value NOT NULL
        );'.
statement := connection prepare: 'INSERT INTO data VALUES("Version", ?1);'.
statement execute: #(20201125).
statement finalize

which makes me think the problem is when binding/clearing string parameters.

gcorriga commented 2 years ago

Changing SQLite3Library>>#with:at:putString: to send 0 instead of -1 when sending #apiBindString:atColumn:with:with:with: seems to avoid the crash.

According to the SQLite3 docs ( https://www.sqlite.org/c3ref/c_static.html ) , 0 corresponds to the SQLITE3_STATIC constant while -1 maps to SQLITE_TRANSIENT. If we were to use SQLITE_STATIC we would need to handle the lifetime of the UTF8-encoded string on the Pharo side of things.

astares commented 1 year ago

Can not reproduce:

Pharo10.0.0 Build information: Pharo-10.0.0+build.538.sha.179ef655ae2b150009a860b127aa3605366659ca (64 Bit) Unnamed

Virtual Machine

C:\Users\astar\Documents\Pharo\vms\100-x64\Pharo.exe CoInterpreter VMMaker-tonel.1 uuid: 58d65b31-a4ab-0d00-a75e-f69302f9ba6c Dec 7 2022 StackToRegisterMappingCogit VMMaker-tonel.1 uuid: 58d65b31-a4ab-0d00-a75e-f69302f9ba6c Dec 7 2022 v9.0.21 - Commit: 191b5a08 - Date: 2022-12-07 20:24:02 +0100

Pharo 9.0.21 built on Dec 7 2022 21:09:41 Compiler: 4.2.1 Compatible Clang 8.0.1 (tags/RELEASE_801/final) VMMaker versionString v9.0.21 - Commit: 191b5a08 - Date: 2022-12-07 20:24:02 +0100 CoInterpreter VMMaker-tonel.1 uuid: 58d65b31-a4ab-0d00-a75e-f69302f9ba6c Dec 7 2022 StackToRegisterMappingCogit VMMaker-tonel.1 uuid: 58d65b31-a4ab-0d00-a75e-f69302f9ba6c Dec 7 2022

image

PLEASE UPDATE VM TO THE LATEST

astares commented 1 year ago

@gcorriga Is this still an issue?

gcorriga commented 1 year ago

@gcorriga Is this still an issue?

This was fixed by PR #54 .

astares commented 1 year ago

Then we can close