BIND 10 #326: Race condition in first startup (when database file does not exist)

[BIND 10 Development]

#326: Race condition in first startup (when database file does not exist)
-------------------------------------+-------------------------------------
                   Reporter:  jelte  |                 Owner:  stephen
                       Type:         |                Status:  reviewing
  defect                             |             Milestone:
                   Priority:  major  |  Sprint-20110830
                  Component:  data   |            Resolution:
  source                             |             Sensitive:  0
                   Keywords:         |           Sub-Project:  DNS
            Defect Severity:         |  Estimated Difficulty:  8
  Medium                             |           Total Hours:  0
Feature Depending on Ticket:         |
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------
Changes (by jelte):

 * owner:  jelte => stephen


Comment:

 Replying to [comment:12 stephen]:
 > '''src/lib/datasrc/sqlite3_accessor.cc'''
 > '''src/lib/datasrc/sqlite3_datasrc.cc'''
 >
 > ''check_schema_version()''
 > The check
 > {{{
 > } else if (rc != SQLITE_BUSY || i == 50) {
 > }}}
 > ... inside the loop is wrong.  The "i == 50" condition will never be
 true as the loop termination condition is "i < 50".
 >

 ah, doh.

 I have a quote configured as an entry message in a chat room;

 "There are two hard problems in computer science: cache invalidation,
 naming things, and off-by-one errors."

 ...

 fixed, made it loop from 1 while <=, instead of < though, so the ==
 WAIT_TIME is more recognizable (as opposed to == WAIT_TIME - 1)

 >
 > "int rc" can be declared inside the loop and initialized with the call
 to sqlite3_exec().
 >

 ack

 > If the schema exists, the code returns the version but does not cancel
 the transaction started by executing "BEGIN EXCLUSIVE TRANSACTION". It
 should execute a rollback (or commit) before returning.
 >

 ack. Also added it in case the creation fails for whatever reason (it may
 not matter much in such a case but still, not nice to leave it open).

 > ''checkAndSetupSchema()''
 > If the schema does exist but the version is not equal to that expected,
 the code will attempt to create the database.  It should only create the
 database if the returned value from check_schema_version() is -1 (table
 does not exist); in all other cases it should report a version mismatch
 and terminate the program.

 crap, of course. Fixed

 >
 > ''general''
 > The wait period (number of iterations of loops) should be symbolic
 constants.
 >

 sure

 > Not part of this ticket, but indenting the continuation lines of each
 statement in SCHEMA_LIST would make it more readable.
 >

 sure

 > ''src/lib/python/isc/datasrc/sqlite3_ds.py''
 > Would it be possible to encapsulate the creation of the database into a
 Python-callable C++ module so that the creation code (also present in
 sqlite3_xxx.cc) is located in just one place?  At the moment the
 possibility exists of divergence between the Python and C++ creation
 functions.

 Yes, not to mention that we right now have 3 versions :p

 But I think that is the ultimate goal for after the datasource refactoring
 (doing everything through datasource wrapper), but right now that doesn't
 help us get this fixed :) I don't see much advantage of having just a
 subset like the creation wrapped, tbh.

-- 
Ticket URL: <http://bind10.isc.org/ticket/326#comment:13>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development