It seems like a programmer error if a statement remains unfinalized when
the DB is closed.  It would be great if something about the API would
help programmers detect and fix errors like this.  Having sqlite3_close
return an error helped a little, because it made you aware there was a
problem, but it didn't help much in finding the problem.

I suspect that if you make sqlite3_close automatically finalize all
statements it'll cause more problems for programmers that might still be
hanging on to those unfinalized statements.

Could you do something in between, where sqlite3_close does close the
file and puts all the statements in some kind of "error" state, but
which doesn't free all the memory for the unfinalized statements?  Then
you could safely and reliably report MISUSE later if the statement was
accessed.

It would also be a nice touch if sqlite3_close could report on the
unfinalized statements, even just getting the SQL text used to create
the statement would be a huge help in tracking down these leaks.  Or add
an additional API to report on all unfinalized statements that could be
called when this sort of error occurs?

                                                --Bob

-----Original Message-----
From: [EMAIL PROTECTED]
[mailto:[EMAIL PROTECTED] On Behalf Of D. Richard Hipp
Sent: Tuesday, May 13, 2008 5:22 PM
To: General Discussion of SQLite Database
Subject: Re: [sqlite] Proposed SQLite C/C++ interface behavior change.


On May 13, 2008, at 8:02 PM, Scott Hess wrote:

> On Tue, May 13, 2008 at 4:51 PM, D. Richard Hipp <[EMAIL PROTECTED]>
> wrote:
>> The currently documented behavior of sqlite3_close() is that when it 
>> called on a database connection that has unfinalized prepared 
>> statements is to return SQLITE_BUSY and fail to close the connection.
>> The rational is that we did not want a call to sqlite3_close() to 
>> destroy sqlite3_stmt* pointers out from under other subsystems. But 
>> for version 3.6.0 we are considering a behavior change in which a 
>> call to sqlite3_close() will silently and automatically call
>> sqlite3_finalize() on all outstanding prepared statements.
>>
>> This is, technically, an incompatible change and we strive to avoid 
>> incompatible changes. But we think it unlikely that this change will 
>> cause any problems, and in fact we suspect it will likely fix more 
>> bugs than it will induce. But before we move forward, it seems good 
>> to submit the idea to the community of SQLite users and programmers.
>>
>> Does anybody have any thoughts on this proposed behavior changes for 
>> the sqlite3_close() interface?
>
> What will happen if you call things like this:
>
> db = sqlite3_open(...);
> sqlite3_prepare_v2(db, zSql, -1, &stmt, NULL); // ...
> sqlite3_close(db);
> sqlite3_finalize(stmt);
>
> ??? If sqlite3_finalize() will succeed, then this seems pretty 
> reasonable to me. If sqlite3_finalize() will fail with a segfault or 
> somesuch, that may be dangerous.

The current behavior in the above is that sqlite3_close() will return
SQLITE_BUSY and then the application will leak memory and file
descriptors because the database connection is never closed.

The proposed new behavior is that the stmt object would be destroyed
(and its memory passed to free()) by the sqlite3_close() call.  We could
arrange for sqlite3_finalize() to attempt to return SQLITE_MISUSE, but
if memory were reused in between the close and the finalize, there is a
small but non-zero chance that the misuse could go undetected and
segfault could result.  Or if this were on android and memory used by
the stmt object gets unmapped, you might segfault with high probability.

>
> My rational is that if someone is building a system which is 
> abstracting SQLite, then it may be that the database handles and the 
> statement handles will not have clear clean-up ordering (for instance 
> in a garbage- collected system). One _could_ go ahead and create such 
> an ordering by using refcounting or whatever is appropriate in your 
> system, but it would also seem reasonable for SQLite to manage this.
>
> This may also come up with multi-threaded systems, where it could be 
> challenging to convince another thread to finalize statements before 
> calling close.
>
> I don't suggest that you should be able to do anything interesting 
> with the orphaned statement handle at this point, other than 
> finalizing it. I suppose it would be nice if all statement-handle 
> functions would start throwing SQLITE_MISUSE or other appropriate 
> error code.
>

Perhaps a better approach would be to modify sqlite3_close() to return
SQLITE_MISUSE if called with unfinalized prepared statements and also
fail and assert() in that case.  That way, applications would crash
during development so that the bugs would be found more quickly, but
after delivery (when assert() is usually turned off) would merely leak
memory and file descriptors but would otherwise continue functioning.

By similar reasoning, perhaps SQLite should always assert(0) in places
where it returns SQLITE_MISUSE.  This is an abuse of assert() since
assert() is only suppose to fail due to internal inconsistencies, not
misuse by the caller.  But it would get the point across.

D. Richard Hipp
[EMAIL PROTECTED]



_______________________________________________
sqlite-users mailing list
sqlite-users@sqlite.org
http://sqlite.org:8080/cgi-bin/mailman/listinfo/sqlite-users
_______________________________________________
sqlite-users mailing list
sqlite-users@sqlite.org
http://sqlite.org:8080/cgi-bin/mailman/listinfo/sqlite-users

Reply via email to