[Asterisk-code-review] func odbc: Check connection status before executing queries. (asterisk[master])

[Anonymous Coward]

Anonymous Coward #1000019 has submitted this change and it was merged.

Change subject: func_odbc: Check connection status before executing queries.
......................................................................


func_odbc: Check connection status before executing queries.

A recent change to func_odbc made it so that a single connection was
maintained per DSN. The problem was that the code was optimistic about
the health of the connection after initially opening it and did nothing
to re-connect in case the connection had died.

This change adds a check before executing a query to ensure that the
connection to the database is still up and running.

ASTERISK-25963 #close
Reported by Ross Beer

Change-Id: Id33c86eb04ff48ca088bb2e3086c27b3b683491d
---
M funcs/func_odbc.c
1 file changed, 81 insertions(+), 5 deletions(-)

Approvals:
  Anonymous Coward #1000019: Verified
  Joshua Colp: Looks good to me, approved
  George Joseph: Looks good to me, but someone else must approve



diff --git a/funcs/func_odbc.c b/funcs/func_odbc.c
index ca15d70..d8ffd90 100644
--- a/funcs/func_odbc.c
+++ b/funcs/func_odbc.c
@@ -243,6 +243,42 @@
 	return dsn;
 }
 
+static SQLHSTMT silent_execute(struct odbc_obj *obj, void *data);
+
+/*!
+ * \brief Determine if the connection has died.
+ *
+ * \param connection The connection to check
+ * \retval 1 Yep, it's dead
+ * \retval 0 It's alive and well
+ */
+static int connection_dead(struct odbc_obj *connection)
+{
+	SQLINTEGER dead;
+	SQLRETURN res;
+	SQLHSTMT stmt;
+
+	if (!connection) {
+		return 1;
+	}
+
+	res = SQLGetConnectAttr(connection->con, SQL_ATTR_CONNECTION_DEAD, &dead, 0, 0);
+	if (SQL_SUCCEEDED(res)) {
+		return dead == SQL_CD_TRUE ? 1 : 0;
+	}
+
+	/* If the Driver doesn't support SQL_ATTR_CONNECTION_DEAD do a direct
+	 * execute of a probing statement and see if that succeeds instead
+	 */
+	stmt = ast_odbc_direct_execute(connection, silent_execute, "SELECT 1");
+	if (!stmt) {
+		return 1;
+	}
+
+	SQLFreeHandle(SQL_HANDLE_STMT, stmt);
+	return 0;
+}
+
 /*!
  * \brief Retrieve a DSN, or create it if it does not exist.
  *
@@ -271,7 +307,26 @@
 		return NULL;
 	}
 
-	ao2_lock(dsn->connection);
+	ao2_lock(dsn);
+	if (!dsn->connection) {
+		dsn->connection = ast_odbc_request_obj(name, 0);
+		if (!dsn->connection) {
+			ao2_unlock(dsn);
+			ao2_ref(dsn, -1);
+			return NULL;
+		}
+		return dsn;
+	}
+
+	if (connection_dead(dsn->connection)) {
+		ast_odbc_release_obj(dsn->connection);
+		dsn->connection = ast_odbc_request_obj(name, 0);
+		if (!dsn->connection) {
+			ao2_unlock(dsn);
+			ao2_ref(dsn, -1);
+			return NULL;
+		}
+	}
 
 	return dsn;
 }
@@ -288,7 +343,7 @@
 		return NULL;
 	}
 
-	ao2_unlock(dsn->connection);
+	ao2_unlock(dsn);
 	ao2_ref(dsn, -1);
 
 	return NULL;
@@ -323,7 +378,16 @@
 	ast_free(result);
 }
 
-static SQLHSTMT generic_execute(struct odbc_obj *obj, void *data)
+/*!
+ * \brief Common execution function for SQL queries.
+ *
+ * \param obj DB connection
+ * \param data The query to execute
+ * \param silent If true, do not print warnings on failure
+ * \retval NULL Failed to execute query
+ * \retval non-NULL The executed statement
+ */
+static SQLHSTMT execute(struct odbc_obj *obj, void *data, int silent)
 {
 	int res;
 	char *sql = data;
@@ -337,7 +401,7 @@
 
 	res = SQLExecDirect(stmt, (unsigned char *)sql, SQL_NTS);
 	if ((res != SQL_SUCCESS) && (res != SQL_SUCCESS_WITH_INFO) && (res != SQL_NO_DATA)) {
-		if (res == SQL_ERROR) {
+		if (res == SQL_ERROR && !silent) {
 			int i;
 			SQLINTEGER nativeerror=0, numfields=0;
 			SQLSMALLINT diagbytes=0;
@@ -354,7 +418,9 @@
 			}
 		}
 
-		ast_log(LOG_WARNING, "SQL Exec Direct failed (%d)![%s]\n", res, sql);
+		if (!silent) {
+			ast_log(LOG_WARNING, "SQL Exec Direct failed (%d)![%s]\n", res, sql);
+		}
 		SQLCloseCursor(stmt);
 		SQLFreeHandle(SQL_HANDLE_STMT, stmt);
 		return NULL;
@@ -363,6 +429,16 @@
 	return stmt;
 }
 
+static SQLHSTMT generic_execute(struct odbc_obj *obj, void *data)
+{
+	return execute(obj, data, 0);
+}
+
+static SQLHSTMT silent_execute(struct odbc_obj *obj, void *data)
+{
+	return execute(obj, data, 1);
+}
+
 /*
  * Master control routine
  */

-- 
To view, visit https://gerrit.asterisk.org/2716
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Id33c86eb04ff48ca088bb2e3086c27b3b683491d
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Mark Michelson <mmichelson at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>