BIND 10 #1253: Datasource refactor finishing touch 3: factory/containers and wrappers

[BIND 10 Development]

#1253: Datasource refactor finishing touch 3: factory/containers and wrappers
-------------------------------------+-------------------------------------
                   Reporter:  jelte  |                 Owner:  vorner
                       Type:  task   |                Status:  reviewing
                   Priority:  minor  |             Milestone:
                  Component:  data   |  Sprint-20111011
  source                             |            Resolution:
                   Keywords:         |             Sensitive:  0
            Defect Severity:  N/A    |           Sub-Project:  DNS
Feature Depending on Ticket:         |  Estimated Difficulty:  3
        Add Hours to Ticket:  0      |           Total Hours:  0
                  Internal?:  0      |
-------------------------------------+-------------------------------------
Changes (by jelte):

 * owner:  jelte => vorner


Comment:

 Replying to [comment:7 vorner]:
 > Hello
 >
 > First, I fixed a problem in Makefile, it failed with `make -j6` here (as
 the library was not yet made).
 >

 Thanks

 > Secondly, I still have trouble with it, it fails the tests:
 > {{{
 > isc.datasrc.Error: Failed to create DataSourceClient of type
 sqlite3:/home/vorner/testing/bind10/lib/sqlite3_ds.so: undefined symbol:
 _ZNK3isc9Exception4whatEv
 > I have a faint memory that gentoo linker is set to strip symbols that
 are not used when loading dynamic libraries or something and maybe the
 python test doesn't use the symbol, so it gets lost or something. But
 that's just a wild guess.
 >

 There is a more general issue that python does weird stuff to the symbol
 tables when it loads libraries.
 Does the sqlite3_ds.so end up being linked to libcc? I did find another
 problem where on solaris it couldn't find the .so in the first place.

 > The stuff in a8a8ceb589f9f3bf4da29717eec446cb2766032c looks strange.
 What happened when it threw? Because this is both incomplete (there's
 infinitely more unhandled exceptions), wrong (as something that inherits
 from DataSourceError can be thrown, in which case this loses information)
 and woodoo-looking.
 >

 The problem is that if a dlopened() module throws an exception, and that
 exception is (partly) defined within that library (including exception
 subclasses that don't override all methods of their parents, whether or
 not these subclasses are defined outside the lib itself), by the time the
 exception handler gets it, the library may be unloaded already (and due to
 the nice and clean RAII pattern, this will always be the case if the
 constructor fails), and it will try to look up type information in a
 library that no longer exists, resulting in undefined behaviour (or most
 probably, segfaults when trying to read or destroy the exception object).

 So that's why i said we need to restrict by contract which exceptions are
 thrown. But given some other issues i saw, perhaps it is much better to
 restrict it even further, and not let createInstance throw anything, ever.

 So here's another approach; i've added a std:string& error argument to the
 createInstance prototype, and they may set some error to it and return
 NULL if there was a problem. The caller still has a high-level catch
 around it (a half-useless exception is better than a segfault with little
 info imo). Currently the createInstance, eh, instances don't make much
 difference in what they catch themselves, but we can always add those. And
 if the result is NULL, the caller will throw the exception.

 This also meant there was no distinction between DataSourceConfigError and
 DataSourceError anymore, so i removed the first (we can use it again if we
 want to do config check *before* initialization)

 Of course we *could* also use a global registry thingy and not unload any
 library until the very end of the whole program. But ew.

 > I'm thinking if we except our iterators, etc, to hold a shared pointer
 to the parent data source client. Maybe not, so in which case your
 approach might be nice (because python programmers don't expect such
 attacks from the language, C++ ones are already used).
 >
 > And a question, does anything use the factory in some way already? Or we
 need to update different branches?
 >

 No, it's not used yet. AFAIK, only jinmei's current branch for 1261 uses a
 slightly different way of initializing the DataSourceClient object, but
 that shouldn't be a big change (same info, just in a different object).

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