From eea6d549f67f2e308f3b1b979dab405876f4143f Mon Sep 17 00:00:00 2001
From: Olof hagsand <olof@hagsand.se>
Date: Thu, 28 Oct 2021 14:49:33 +0200
Subject: [PATCH] List pagination locking for state-data changes

Changed semantics of locking: instead of relying of locking running-db,
an automatic lock bound to a session is maintained. When the session closes
the lock is released.
---
 apps/backend/backend_get.c                | 13 +++++++++----
 apps/backend/clixon_backend_plugin.h      |  5 +++++
 apps/backend/clixon_backend_transaction.c | 17 +++++++++--------
 apps/cli/cli_show.c                       |  6 ------
 example/main/example_backend.c            |  3 ++-
 5 files changed, 25 insertions(+), 19 deletions(-)

diff --git a/apps/backend/backend_get.c b/apps/backend/backend_get.c
index 1d814516..54042985 100644
--- a/apps/backend/backend_get.c
+++ b/apps/backend/backend_get.c
@@ -592,11 +592,16 @@ get_list_pagination(clicon_handle        h,
 #endif
     }
     else {/* Check if running locked (by this session) */
-	if ((iddb = xmldb_islocked(h, "running")) != 0 &&
-	    iddb == ce->ce_id)
+	if ((iddb = xmldb_islocked(h, PAGINATE_STATE_LOCK)) != 0){
+	    if (iddb == ce->ce_id)
+		locked = 1;
+	    else
+		locked = 0;
+	}
+	else {
+	    xmldb_lock(h, PAGINATE_STATE_LOCK, ce->ce_id);
 	    locked = 1;
-	else
-	    locked = 0;
+	}
 	if ((ret = clixon_pagination_cb_call(h, xpath, locked,
 					     offset, limit,
 					     xret)) < 0)
diff --git a/apps/backend/clixon_backend_plugin.h b/apps/backend/clixon_backend_plugin.h
index f88bb41f..c6ab9177 100644
--- a/apps/backend/clixon_backend_plugin.h
+++ b/apps/backend/clixon_backend_plugin.h
@@ -38,6 +38,11 @@
 #ifndef _CLIXON_BACKEND_PLUGIN_H_
 #define _CLIXON_BACKEND_PLUGIN_H_
 
+/*
+ * Constants
+ */
+#define PAGINATE_STATE_LOCK "paginate-state-lock"
+
 /*
  * Types
  */
diff --git a/apps/backend/clixon_backend_transaction.c b/apps/backend/clixon_backend_transaction.c
index 0a7889ce..7c718a63 100644
--- a/apps/backend/clixon_backend_transaction.c
+++ b/apps/backend/clixon_backend_transaction.c
@@ -309,15 +309,16 @@ pagination_limit(pagination_data pd)
     return ((pagination_data_t *)pd)->pd_limit;
 }
 
-/*! Get pagination data: locked parameter
+/*! Get pagination data: locked parameter for state data
  *
- * Pagination can use a lock/transaction mechanism 
- * If locking is not used, the plugin cannot expect more pagination calls, and no state or 
- * caching should be used
- * If locking is used, the pagination is part of a session transaction and the plugin may cache
- * state (such as a cache) and can expect more pagination calls until the running db-lock is 
- * released, (see ca_lockdb)
- * The transaction is the regular lock/unlock db of running-db of a specific session.
+ * A "paginate" lock is set for a specific session when list-pagination is in progress.
+ * If locking is set, the pagination is part of a session transaction and the plugin may 
+ * cache state (such as a cache) and can expect more pagination calls until the "paginate"
+ * db-lock is released.
+ *
+ * If you want to do the same for config data, it is recommended to use lock-db on
+ * the appropriate database, but this risks of indefinite blocking, as long as the
+ * session endures.
  * @param[in]  pd     Pagination userdata
  * @retval     locked 0: unlocked/stateless 1: locked by this caller
  */
diff --git a/apps/cli/cli_show.c b/apps/cli/cli_show.c
index 07ba04ec..6f7f26a0 100644
--- a/apps/cli/cli_show.c
+++ b/apps/cli/cli_show.c
@@ -919,7 +919,6 @@ cli_pagination(clicon_handle h,
     uint32_t         limit = 0;
     cxobj          **xvec = NULL;
     size_t           xlen;
-    int              locked = 0;
     
     if (cvec_len(argv) != 5){
 	clicon_err(OE_PLUGIN, 0, "Expected usage: <xpath> <prefix> <namespace> <format> <limit>");
@@ -949,9 +948,6 @@ cli_pagination(clicon_handle h,
     }
     if ((nsc = xml_nsctx_init(prefix, namespace)) == NULL)
 	goto done;
-    if (clicon_rpc_lock(h, "running") < 0)
-	goto done;
-    locked++;
     for (i = 0;; i++){
 	if (clicon_rpc_get_pageable_list(h, "running", xpath, nsc,
 					 CONTENT_ALL,
@@ -1004,8 +1000,6 @@ cli_pagination(clicon_handle h,
     } /* for i */
     retval = 0;
  done:
-    if (locked)
-	clicon_rpc_unlock(h, "running");
     if (xvec)
 	free(xvec);
     if (xret)
diff --git a/example/main/example_backend.c b/example/main/example_backend.c
index 947d9d70..6d5655d2 100644
--- a/example/main/example_backend.c
+++ b/example/main/example_backend.c
@@ -545,6 +545,7 @@ example_statefile(clicon_handle     h,
  * @param[in]  xpath    Registered XPath using canonical prefixes
  * @param[in]  userargs Per-call user arguments
  * @param[in]  arg      Per-path user argument
+ * @see example_lockdb
  */
 int 
 example_pagination(void            *h0,
@@ -667,7 +668,7 @@ example_lockdb(clicon_handle h,
 
     /* Part of cached pagination example
      */
-    if (strcmp(db, "running") == 0 && lock == 0 &&
+    if (strcmp(db, PAGINATE_STATE_LOCK) == 0 && lock == 0 &&
 	_state && _state_file && _state_file_cached && _state_file_transaction){
 	if (_state_xml_cache){
 	    xml_free(_state_xml_cache);