Skip to content

Commit

Permalink
Finalize statements before closing connection (#296)
Browse files Browse the repository at this point in the history
* Try to fix #292 by disconnecting before freeing

* Finalize statements on connection before closing

Fixes #292. This was a bit of a bear to track down, mainly
because the docs for SQLDisconnect state explicitly that all statements
on the connection will be closed/freed when disconnecting. For whatever reason, that doesn't seem to be the case on windows, because when we keep a `WeakKeyDict` of statement handles and finalize them all before closing the connection, we no longer have issues. ¯\_(ツ)_/¯
  • Loading branch information
quinnj authored Jun 6, 2020
1 parent a6d11ac commit 093d0b4
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 1 deletion.
3 changes: 3 additions & 0 deletions src/API.jl
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,9 @@ mutable struct Handle
h = new(type, ptr)
finalizer(h) do x
if x.ptr != C_NULL
if x.type == SQL_HANDLE_DBC
SQLDisconnect(x.ptr)
end
SQLFreeHandle(x.type, x.ptr)
x.ptr = C_NULL
end
Expand Down
10 changes: 9 additions & 1 deletion src/dbinterface.jl
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,10 @@ mutable struct Connection <: DBInterface.Connection
types::Dict{Type, String}
alltypes::Any
cursorstmt::Any # keep track of most recent stmt cursor to free appropriately before next execute
stmts::WeakKeyDict{Any, Any}
end

Connection(dbc::API.Handle, dsn) = Connection(dbc, dsn, '\0', Dict{Type, String}(), nothing, nothing)
Connection(dbc::API.Handle, dsn) = Connection(dbc, dsn, '\0', Dict{Type, String}(), nothing, nothing, WeakKeyDict())

Base.show(io::IO, conn::Connection) = print(io, "ODBC.Connection($(conn.dsn))")

Expand Down Expand Up @@ -67,6 +68,11 @@ Base.isopen(c::Connection) = c.dbc.ptr != C_NULL && API.getconnectattr(c.dbc, AP

"disconnect a connected `Connection`"
function disconnect!(conn::Connection)
# for #292, it seems we need to finalize any stmts on the connection
# before disconnect/freehandle, even though disconnect should do that for us
for stmt in keys(conn.stmts)
finalize(stmt)
end
API.disconnect(conn.dbc)
return nothing
end
Expand Down Expand Up @@ -108,6 +114,7 @@ and re-using it many times with different parameters (or the same) efficiently.
function DBInterface.prepare(conn::Connection, sql::AbstractString)
clear!(conn)
stmt = API.prepare(conn.dbc, sql)
conn.stmts[stmt] = 0
return Statement(conn, stmt, sql, nothing, API.numparams(stmt))
end

Expand Down Expand Up @@ -162,6 +169,7 @@ have more benefit for repeated executions (even with different parameters).
function DBInterface.execute(conn::Connection, sql::AbstractString, params=(); debug::Bool=false, kw...)
clear!(conn)
stmt = API.Handle(API.SQL_HANDLE_STMT, API.getptr(conn.dbc))
conn.stmts[stmt] = 0
conn.cursorstmt = stmt
API.enableasync(stmt)
bindings = bindparams(stmt, params, nothing)
Expand Down

0 comments on commit 093d0b4

Please sign in to comment.