Browse Source

[2051] A trick to hold the dsrc_client_ alive

We have a class to keep a reference to it. This is in C++ only yet, the
python part needs to be implemented.
Michal 'vorner' Vaner 12 years ago
parent
commit
77ac69b41c

+ 37 - 2
src/lib/datasrc/client_list.cc

@@ -167,6 +167,39 @@ ConfigurableClientList::configure(const Element& config, bool allow_cache) {
     }
 }
 
+namespace {
+
+class CacheKeeper : public ClientList::FindResult::LifeKeeper {
+public:
+    CacheKeeper(const boost::shared_ptr<InMemoryClient>& cache) :
+        cache_(cache)
+    {}
+private:
+    const boost::shared_ptr<InMemoryClient> cache_;
+};
+
+class ContainerKeeper : public ClientList::FindResult::LifeKeeper {
+public:
+    ContainerKeeper(const DataSourceClientContainerPtr& container) :
+        container_(container)
+    {}
+private:
+    const DataSourceClientContainerPtr container_;
+};
+
+boost::shared_ptr<ClientList::FindResult::LifeKeeper>
+genKeeper(const ConfigurableClientList::DataSourceInfo& info) {
+    if (info.cache_) {
+        return (boost::shared_ptr<ClientList::FindResult::LifeKeeper>(
+            new CacheKeeper(info.cache_)));
+    } else {
+        return (boost::shared_ptr<ClientList::FindResult::LifeKeeper>(
+            new ContainerKeeper(info.container_)));
+    }
+}
+
+}
+
 ClientList::FindResult
 ConfigurableClientList::find(const dns::Name& name, bool want_exact_match,
                             bool) const
@@ -185,10 +218,11 @@ ConfigurableClientList::find(const dns::Name& name, bool want_exact_match,
         ZoneFinderPtr finder;
         uint8_t matched_labels;
         bool matched;
+        boost::shared_ptr<FindResult::LifeKeeper> keeper;
         operator FindResult() const {
             // Conversion to the right result. If we return this, there was
             // a partial match at best.
-            return (FindResult(datasrc_client, finder, false));
+            return (FindResult(datasrc_client, finder, false, keeper));
         }
     } candidate;
 
@@ -206,7 +240,7 @@ ConfigurableClientList::find(const dns::Name& name, bool want_exact_match,
                 // TODO: In case we have only the datasource and not the finder
                 // and the need_updater parameter is true, get the zone there.
                 return (FindResult(client, result.zone_finder,
-                                   true));
+                                   true, genKeeper(info)));
             case result::PARTIALMATCH:
                 if (!want_exact_match) {
                     // In case we have a partial match, check if it is better
@@ -230,6 +264,7 @@ ConfigurableClientList::find(const dns::Name& name, bool want_exact_match,
                         candidate.finder = result.zone_finder;
                         candidate.matched_labels = labels;
                         candidate.matched = true;
+                        candidate.keeper = genKeeper(info);
                     }
                 }
                 break;

+ 23 - 4
src/lib/datasrc/client_list.h

@@ -65,15 +65,27 @@ public:
     /// Instead, all the member variables are defined as const and can be
     /// accessed directly.
     struct FindResult {
+        /// \brief Internal class for holding a reference.
+        ///
+        /// This is used to make sure the data source client isn't released
+        /// too soon.
+        ///
+        /// \see life_keeper_;
+        class LifeKeeper {
+        public:
+            virtual ~LifeKeeper() {};
+        };
         /// \brief Constructor.
         ///
         /// It simply fills in the member variables according to the
         /// parameters. See the member descriptions for their meaning.
         FindResult(DataSourceClient* dsrc_client, const ZoneFinderPtr& finder,
-                   bool exact_match) :
+                   bool exact_match,
+                   const boost::shared_ptr<LifeKeeper>& life_keeper) :
             dsrc_client_(dsrc_client),
             finder_(finder),
-            exact_match_(exact_match)
+            exact_match_(exact_match),
+            life_keeper_(life_keeper)
         {}
 
         /// \brief Negative answer constructor.
@@ -101,8 +113,9 @@ public:
         /// If no such data source exists, this is NULL pointer.
         ///
         /// Note that the pointer is valid only as long the ClientList which
-        /// returned the pointer is alive and was not reconfigured. The
-        /// ownership is preserved within the ClientList.
+        /// returned the pointer is alive and was not reconfigured or you hold
+        /// a reference to life_keeper_. The ownership is preserved within the
+        /// ClientList.
         DataSourceClient* const dsrc_client_;
 
         /// \brief The finder for the requested zone.
@@ -116,6 +129,12 @@ public:
 
         /// \brief If the result is an exact match.
         const bool exact_match_;
+
+        /// \brief Something that holds the dsrc_client_ valid.
+        ///
+        /// As long as you hold the life_keeper_, the dsrc_client_ is
+        /// guaranteed to be valid.
+        const boost::shared_ptr<LifeKeeper> life_keeper_;
     };
 
     /// \brief Search for a zone through the data sources.

+ 9 - 0
src/lib/datasrc/tests/client_list_unittest.cc

@@ -245,6 +245,12 @@ public:
         ASSERT_NE(ZoneFinderPtr(), result.finder_);
         EXPECT_EQ(name, result.finder_->getOrigin());
         EXPECT_EQ(exact, result.exact_match_);
+        // If it is a positive result, there's something to keep
+        // alive, even when we don't know what it is.
+        // Any better idea how to test it actually keeps the thing
+        // alive?
+        EXPECT_NE(shared_ptr<ClientList::FindResult::LifeKeeper>(),
+                  result.life_keeper_);
         if (from_cache) {
             EXPECT_NE(shared_ptr<InMemoryZoneFinder>(),
                       dynamic_pointer_cast<InMemoryZoneFinder>(
@@ -315,6 +321,9 @@ TEST_F(ListTest, selfTest) {
     EXPECT_EQ(result::NOTFOUND, ds_[1]->findZone(Name("example.org")).code);
     EXPECT_EQ(result::NOTFOUND, ds_[0]->findZone(Name("aaa")).code);
     EXPECT_EQ(result::NOTFOUND, ds_[0]->findZone(Name("zzz")).code);
+    // Nothing to keep alive here.
+    EXPECT_EQ(shared_ptr<ClientList::FindResult::LifeKeeper>(),
+                  negativeResult_.life_keeper_);
 }
 
 // Test the list we create with empty configuration is, in fact, empty