Browse Source

[1206] use RAII for ldopened lib itself too

Jelte Jansen 13 years ago
parent
commit
0372723794
3 changed files with 48 additions and 31 deletions
  1. 32 28
      src/lib/datasrc/factory.cc
  2. 13 3
      src/lib/datasrc/factory.h
  3. 3 0
      src/lib/datasrc/tests/factory_unittest.cc

+ 32 - 28
src/lib/datasrc/factory.cc

@@ -27,46 +27,50 @@ using namespace isc::datasrc;
 namespace isc {
 namespace datasrc {
 
-DataSourceClientContainer::DataSourceClientContainer(const std::string& type,
-                                                     ConstElementPtr config)
-{
-    // The name of the loadable module is type + _ds.so
-    // config is assumed to be ok
-    std::string dl_name = type + "_ds.so";
 
-    ds_lib = dlopen(dl_name.c_str(), RTLD_NOW | RTLD_LOCAL);
-    if (ds_lib == NULL) {
-        isc_throw(DataSourceError, "Unable to load " << type <<
+DLHolder::DLHolder(const std::string& name) : ds_name_(name) {
+    ds_lib_ = dlopen(ds_name_.c_str(), RTLD_NOW | RTLD_LOCAL);
+    if (ds_lib_ == NULL) {
+        isc_throw(DataSourceError, "Unable to load " << ds_name_ <<
                   ": " << dlerror());
     }
+}
+
+DLHolder::~DLHolder() {
+    dlclose(ds_lib_);
+}
+
+void*
+DLHolder::getSym(const char* name) {
+    // Since dlsym can return NULL on success, we check for errors by
+    // first clearing any existing errors with dlerror(), then calling dlsym,
+    // and finally checking for errors with dlerror()
     dlerror();
-    ds_creator* ds_create = (ds_creator*)dlsym(ds_lib, "createInstance");
+
+    void *sym = dlsym(ds_lib_, name);
+
     const char* dlsym_error = dlerror();
     if (dlsym_error != NULL) {
-        dlclose(ds_lib);
-        isc_throw(DataSourceError, "Error in library " << type <<
+        dlclose(ds_lib_);
+        isc_throw(DataSourceError, "Error in library " << ds_name_ <<
                   ": " << dlsym_error);
     }
-    try {
-        instance = ds_create(config);
-    } catch (...) {
-        dlclose(ds_lib);
-        throw;
-    }
 
-    dlerror();
-    destructor = (ds_destructor*)dlsym(ds_lib, "destroyInstance");
-    dlsym_error = dlerror();
-    if (dlsym_error != NULL) {
-        dlclose(ds_lib);
-        isc_throw(DataSourceError, "Error in library " << type <<
-                  ": " << dlsym_error);
-    }
+    return (sym);
+}
+
+DataSourceClientContainer::DataSourceClientContainer(const std::string& type,
+                                                     ConstElementPtr config)
+: ds_lib_(type + "_ds.so")
+{
+    ds_creator* ds_create = (ds_creator*)ds_lib_.getSym("createInstance");
+    destructor_ = (ds_destructor*)ds_lib_.getSym("destroyInstance");
+
+    instance_ = ds_create(config);
 }
 
 DataSourceClientContainer::~DataSourceClientContainer() {
-    destructor(instance);
-    dlclose(ds_lib);
+    destructor_(instance_);
 }
 
 } // end namespace datasrc

+ 13 - 3
src/lib/datasrc/factory.h

@@ -31,15 +31,25 @@ namespace datasrc {
 typedef DataSourceClient* ds_creator(isc::data::ConstElementPtr config);
 typedef void ds_destructor(DataSourceClient* instance);
 
+class DLHolder {
+public:
+    DLHolder(const std::string& name);
+    ~DLHolder();
+    void* getSym(const char* name);
+private:
+    const std::string ds_name_;
+    void *ds_lib_;
+};
+
 class DataSourceClientContainer {
 public:
     DataSourceClientContainer(const std::string& type,
                               isc::data::ConstElementPtr config);
     ~DataSourceClientContainer();
 private:
-    DataSourceClient* instance;
-    ds_destructor* destructor;
-    void *ds_lib;
+    DataSourceClient* instance_;
+    ds_destructor* destructor_;
+    DLHolder ds_lib_;
 };
 
 

+ 3 - 0
src/lib/datasrc/tests/factory_unittest.cc

@@ -120,6 +120,9 @@ TEST(FactoryTest, sqlite3ClientBadConfig3) {
     config->set("file", Element::create("/foo/bar/doesnotexist"));
     ASSERT_THROW(DataSourceClientContainer("sqlite3", config),
                  SQLite3Error);
+
+    config->set("file", Element::create("/tmp/some_file.sqlite3"));
+    DataSourceClientContainer dsc("sqlite3", config);
 }
 } // end anonymous namespace