[#3648] addressed review comments

diff --git a/ChangeLog b/ChangeLog
index df4e2b115950d234e759a7ee7eb97664de11fdc7..f89bc78c3775ba3e2458ec5f646c7af6870a8041 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,11 @@
+2326.  [bug]           razvan
+       Fixed a bug which was causing the allocation engine to reject the
+       lease if a data race caused by a different server updating the
+       shared database entries was detected. The entire packet is now
+       dropped in this particular case. This applies to both kea-dhp4
+       and kea-dhcp6 servers.
+       (Gitlab #3648)
+
 2325.  [bug]           razvan
        Fixed a bug which was causing address allocation counters to be
        negative when client released leases and the server has lease

diff --git a/src/hooks/dhcp/mysql/mysql_lease_mgr.cc b/src/hooks/dhcp/mysql/mysql_lease_mgr.cc
index 8c0dd3179bb4a60f5f642763e7cb48a391d6ccca..186243db21094dc8685300ff7e8c26cc34fdcee1 100644 (file)
--- a/src/hooks/dhcp/mysql/mysql_lease_mgr.cc
+++ b/src/hooks/dhcp/mysql/mysql_lease_mgr.cc
@@ -3266,7 +3266,8 @@ MySqlLeaseMgr::updateLeaseCommon(MySqlLeaseContextPtr& ctx,
     // If no rows affected, lease doesn't exist.
     if (affected_rows == 0) {
         isc_throw(NoSuchLease, "unable to update lease for address " <<
-                  lease->addr_.toText() << " as it does not exist");
+                  lease->addr_.toText() << " either because the lease does not exist, "
+                      "it has been deleted or it has changed in the database.");
     }
 
     // Should not happen - primary key constraint should only have selected

diff --git a/src/hooks/dhcp/pgsql/pgsql_lease_mgr.cc b/src/hooks/dhcp/pgsql/pgsql_lease_mgr.cc
index 2e10f21ec8cc3cfd0abe8021bac5ec46cc0309ea..39a11a73fd2b2819edc9d31df9ad4d94cf3762b2 100644 (file)
--- a/src/hooks/dhcp/pgsql/pgsql_lease_mgr.cc
+++ b/src/hooks/dhcp/pgsql/pgsql_lease_mgr.cc
@@ -2500,7 +2500,8 @@ PgSqlLeaseMgr::updateLeaseCommon(PgSqlLeaseContextPtr& ctx,
     // If no rows affected, lease doesn't exist.
     if (affected_rows == 0) {
         isc_throw(NoSuchLease, "unable to update lease for address " <<
-                  lease->addr_.toText() << " as it does not exist");
+                  lease->addr_.toText() << " either because the lease does not exist, "
+                      "it has been deleted or it has changed in the database.");
     }
 
     // Should not happen - primary key constraint should only have selected

diff --git a/src/lib/dhcpsrv/alloc_engine.cc b/src/lib/dhcpsrv/alloc_engine.cc
index 1294e999e4ec0395dad7ade2590008a81a4189bc..7a4ef316cad08b7867efb32e6f0419ab7868907d 100644 (file)
--- a/src/lib/dhcpsrv/alloc_engine.cc
+++ b/src/lib/dhcpsrv/alloc_engine.cc
@@ -630,8 +630,8 @@ AllocEngine::allocateLeases6(ClientContext6& ctx) {
             return (leases);
         }
 
-    } catch (const NoSuchLease&) {
-        throw;
+    } catch (const NoSuchLease& e) {
+        isc_throw(NoSuchLease, "detected data race in AllocEngine::allocateLeases6: " << e.what());
 
     } catch (const isc::Exception& e) {
 
@@ -2186,8 +2186,8 @@ AllocEngine::renewLeases6(ClientContext6& ctx) {
 
         return (leases);
 
-    } catch (const NoSuchLease&) {
-        throw;
+    } catch (const NoSuchLease& e) {
+        isc_throw(NoSuchLease, "detected data race in AllocEngine::renewLeases6: " << e.what());
 
     } catch (const isc::Exception& e) {
 
@@ -3753,8 +3753,8 @@ AllocEngine::allocateLease4(ClientContext4& ctx) {
             ctx.new_lease_ = requestLease4(ctx);
         }
 
-    } catch (const NoSuchLease&) {
-        throw;
+    } catch (const NoSuchLease& e) {
+        isc_throw(NoSuchLease, "detected data race in AllocEngine::allocateLease4: " << e.what());
 
     } catch (const isc::Exception& e) {
         // Some other error, return an empty lease.

diff --git a/src/lib/dhcpsrv/memfile_lease_mgr.cc b/src/lib/dhcpsrv/memfile_lease_mgr.cc
index 83922033bd3bff66b7a41d6ef131038ab8c03ee2..046cd0bebcc2e9b6331255b883dc41504e4083a1 100644 (file)
--- a/src/lib/dhcpsrv/memfile_lease_mgr.cc
+++ b/src/lib/dhcpsrv/memfile_lease_mgr.cc
@@ -1862,8 +1862,9 @@ Memfile_LeaseMgr::updateLease4Internal(const Lease4Ptr& lease) {
         ((*lease_it)->valid_lft_ != lease->current_valid_lft_))) {
         // For test purpose only: check that the lease has not changed in
         // the database.
-        isc_throw(NoSuchLease, "failed to update the lease with address "
-                  << lease->addr_ << " - lease has changed in database");
+        isc_throw(NoSuchLease, "unable to update lease for address " <<
+                  lease->addr_.toText() << " either because the lease does not exist, "
+                      "it has been deleted or it has changed in the database.");
     }
 
     // Try to write a lease to disk first. If this fails, the lease will
@@ -1925,8 +1926,9 @@ Memfile_LeaseMgr::updateLease6Internal(const Lease6Ptr& lease) {
         ((*lease_it)->valid_lft_ != lease->current_valid_lft_))) {
         // For test purpose only: check that the lease has not changed in
         // the database.
-        isc_throw(NoSuchLease, "failed to update the lease with address "
-                  << lease->addr_ << " - lease has changed in database");
+        isc_throw(NoSuchLease, "unable to update lease for address " <<
+                  lease->addr_.toText() << " either because the lease does not exist, "
+                      "it has been deleted or it has changed in the database.");
     }
 
     // Try to write a lease to disk first. If this fails, the lease will