Fixed transactions support in user management commands

src/jrd/UserManagement.cpp

@@ -26,6 +26,8 @@
 #include "../jrd/common.h"
 #include "../jrd/jrd.h"
 #include "../jrd/jrd_pwd.h"
+#include "../jrd/tra.h"
+#include "../jrd/msg_encode.h"
 #include "../utilities/gsec/gsec.h"
 #include "../utilities/gsec/secur_proto.h"
 
@@ -33,13 +35,13 @@ using namespace Jrd;
 using namespace Firebird;
 
 
-UserManagement::UserManagement(thread_db* tdbb)
+UserManagement::UserManagement(jrd_tra* tra)
-	: database(0), transaction(0)
+	: database(0), transaction(0), commands(*tra->tra_pool)
 {
 	char securityDatabaseName[MAXPATHLEN];
 	SecurityDatabase::getPath(securityDatabaseName);
 	ISC_STATUS_ARRAY status;
-	Attachment* att = tdbb->getAttachment();
+	Attachment* att = tra->tra_attachment;
 
 	ClumpletWriter dpb(ClumpletReader::Tagged, MAX_DPB_SIZE, isc_dpb_version1);
 	dpb.insertByte(isc_dpb_gsec_attach, TRUE);
@@ -61,22 +63,14 @@ UserManagement::UserManagement(thread_db* tdbb)
 	}
 }
 
-void UserManagement::commit()
-{
-	ISC_STATUS_ARRAY status;
-	if (transaction)
-	{
-		// Commit transaction in security database
-		if (isc_commit_transaction(status, &transaction))
-		{
-			status_exception::raise(status);
-		}
-		transaction = 0;
-	}
-}
-
 UserManagement::~UserManagement()
 {
+	for (ULONG i = 0; i < commands.getCount(); ++i)
+	{
+		delete commands[i];
+	}
+	commands.clear();
+
 	ISC_STATUS_ARRAY status;
 	if (transaction)
 	{
@@ -96,13 +90,72 @@ UserManagement::~UserManagement()
 	}
 }
 
-int UserManagement::execute(ISC_STATUS* status, internal_user_data* u)
+void UserManagement::commit()
+{
+	ISC_STATUS_ARRAY status;
+	if (transaction)
+	{
+		// Commit transaction in security database
+		if (isc_commit_transaction(status, &transaction))
+		{
+			status_exception::raise(status);
+		}
+		transaction = 0;
+	}
+}
+
+USHORT UserManagement::put(internal_user_data* userData)
+{
+	size_t ret = commands.getCount();
+	if (ret > MAX_USHORT)
+	{
+		status_exception::raise(Arg::Gds(isc_random) << "Too many user management DDL per transaction)");
+	}
+	commands.push(userData);
+	return ret;
+}
+
+void UserManagement::execute(USHORT id)
 {
 #if (defined BOOT_BUILD || defined EMBEDDED)
 	status_exception::raise(Arg::Gds(isc_wish_list));
-	return 0; // make the compiler happy
 #else
-	return (!u->user_name_entered) ? 
+	if ((!transaction) || (!commands[id]))
-		GsecMsg18 : SECURITY_exec_line(status, database, transaction, u, NULL, NULL);
+	{
+		// Already executed
+		return;
+	}
+
+	if (id >= commands.getCount())
+	{
+		status_exception::raise(Arg::Gds(isc_random) << "Wrong job id passed to UserManagement::execute()");
+	}
+
+	ISC_STATUS_ARRAY status;
+	int errcode = (!commands[id]->user_name_entered) ? GsecMsg18 : 
+		SECURITY_exec_line(status, database, transaction, commands[id], NULL, NULL);
+
+	switch (errcode)
+	{
+	case 0: // nothing
+	    break;
+	case GsecMsg22:
+		status_exception::raise(Arg::Gds(ENCODE_ISC_MSG(errcode, GSEC_MSG_FAC)) <<
+								Arg::Str(commands[id]->user_name));
+	default:
+		status_exception::raise(Arg::Gds(ENCODE_ISC_MSG(errcode, GSEC_MSG_FAC)));
+	}
+
+	delete commands[id];
+	commands[id] = 0;
 #endif
 }
+
+UserManagement* jrd_tra::getUserManagement()
+{
+	if (!tra_user_management)
+	{
+		tra_user_management = FB_NEW(*tra_pool) UserManagement(this);
+	}
+	return tra_user_management;
+}

src/jrd/UserManagement.h

@@ -24,6 +24,7 @@
 #define JRD_USER_MANAGEMENT_H
 
 #include "firebird.h"
+#include "../common/classes/array.h"
 #include "../jrd/ibase.h"
 
 struct internal_user_data;
@@ -31,20 +32,25 @@ struct internal_user_data;
 namespace Jrd {
 
 class thread_db;
+class jrd_tra;
 
 // User management argument for deferred work
 class UserManagement
 {
 public:
-	explicit UserManagement(thread_db* tdbb);
+	explicit UserManagement(jrd_tra* tra);
 	~UserManagement();
 
+	// store userData for DFW-time processing
+	USHORT put(internal_user_data* userData);
+	// execute command with ID
+	void execute(USHORT id);
+	// commit transaction in security database
+	void commit();
+
 private:
 	FB_API_HANDLE database, transaction;
-
+	Firebird::HalfStaticArray<internal_user_data*, 8> commands;
-public:
-	int execute(ISC_STATUS* status, internal_user_data* u);
-	void commit();
 };
 
 }	// namespace

src/jrd/dfw.epp

@@ -378,20 +378,12 @@ DeferredWork::~DeferredWork()
 {
 	if (dfw_args)
 	{
-		if (dfw_type == dfw_user_management)
+		DeferredWork* tmp = dfw_args;
+		while (tmp)
 		{
-			UserManagement* sec = reinterpret_cast<UserManagement*>(dfw_args);
+			DeferredWork* next = tmp->dfw_next;
-			delete sec;
+			delete tmp;
-		}
+			tmp = next;
-		else
-		{
-			DeferredWork* tmp = dfw_args;
-			while (tmp)
-			{
-				DeferredWork* next = tmp->dfw_next;
-				delete tmp;
-				tmp = next;
-			}
 		}
 	}
 
@@ -1260,12 +1252,10 @@ static bool user_management(thread_db*		tdbb,
 	case 2:
 		return true;
 	case 3:
-		// Commit
+		transaction->getUserManagement()->execute(work->dfw_id);
-		sec = reinterpret_cast<UserManagement*>(work->dfw_args);
+		return true;
-		if (sec)
+	case 4:
-		{
+		transaction->getUserManagement()->commit();	// safe to be called multiple times
-			sec->commit();
-		}
 		return false;
 	}
 
@@ -5094,10 +5084,19 @@ static DeferredWork* post_work(jrd_tra* transaction,
 	*ptr = FB_NEW(*transaction->tra_pool)
 		DeferredWork(*transaction->tra_pool, type, id, sav_number, string, length);
 
-	if (type != dfw_post_event)
+	switch (type)
+	{
+	case dfw_user_management:
 		transaction->tra_flags |= TRA_deferred_meta;
-	else if (transaction->tra_save_point)
+		// fall down ...
-		transaction->tra_save_point->sav_flags |= SAV_event_post;
+	case dfw_post_event:
+		if (transaction->tra_save_point)
+			transaction->tra_save_point->sav_flags |= SAV_force_dfw;
+		break;
+	default:
+		transaction->tra_flags |= TRA_deferred_meta;
+		break;
+	}
 
 	return *ptr;
 }

src/jrd/dyn.epp

@@ -2292,15 +2292,7 @@ static void dyn_user(Global* gbl, const UCHAR** ptr)
 	ISC_STATUS_ARRAY status;
 	try
 	{
-		DeferredWork* work = DFW_post_work(tra, dfw_user_management, NULL, 0);
+		internal_user_data* userData = FB_NEW(*tra->tra_pool) internal_user_data;
-		UserManagement* sec = reinterpret_cast<UserManagement*>(work->dfw_args);
-		if (!sec)
-		{
-			sec = FB_NEW(*tra->tra_pool) UserManagement(tdbb);
-			work->dfw_args = reinterpret_cast<DeferredWork*>(sec);
-		}
-
-		internal_user_data userData;
 		UCHAR verb;
 		while ((verb = *(*ptr)++) != isc_user_end)
 		{
@@ -2311,23 +2303,23 @@ static void dyn_user(Global* gbl, const UCHAR** ptr)
 			{
 			case isc_dyn_user_add:
 				text.upper();
-				userData.operation = ADD_OPER;
+				userData->operation = ADD_OPER;
-				text.copyTo(userData.user_name, sizeof(userData.user_name));
+				text.copyTo(userData->user_name, sizeof(userData->user_name));
-				userData.user_name_entered = true;
+				userData->user_name_entered = true;
 				break;
 
 			case isc_dyn_user_mod:
 				text.upper();
-				userData.operation = MOD_OPER;
+				userData->operation = MOD_OPER;
-				text.copyTo(userData.user_name, sizeof(userData.user_name));
+				text.copyTo(userData->user_name, sizeof(userData->user_name));
-				userData.user_name_entered = true;
+				userData->user_name_entered = true;
 				break;
 
 			case isc_dyn_user_del:
 				text.upper();
-				userData.operation = DEL_OPER;
+				userData->operation = DEL_OPER;
-				text.copyTo(userData.user_name, sizeof(userData.user_name));
+				text.copyTo(userData->user_name, sizeof(userData->user_name));
-				userData.user_name_entered = true;
+				userData->user_name_entered = true;
 				break;
 
 			case isc_dyn_user_passwd:
@@ -2336,63 +2328,53 @@ static void dyn_user(Global* gbl, const UCHAR** ptr)
 					// 249: Password should not be empty string
 					status_exception::raise(Arg::Gds(ENCODE_ISC_MSG(249, DYN_MSG_FAC)));
 				}
-				text.copyTo(userData.password, sizeof(userData.password));
+				text.copyTo(userData->password, sizeof(userData->password));
-				userData.password_entered = true;
+				userData->password_entered = true;
 				break;
 
 			case isc_dyn_user_first:
 				if (text.hasData())
 				{
-					text.copyTo(userData.first_name, sizeof(userData.first_name));
+					text.copyTo(userData->first_name, sizeof(userData->first_name));
-					userData.first_name_entered = true;
+					userData->first_name_entered = true;
 				}
 				else
 				{
-					userData.first_name_entered = false;
+					userData->first_name_entered = false;
-					userData.first_name_specified = true;
+					userData->first_name_specified = true;
 				}
 				break;
 
 			case isc_dyn_user_middle:
 				if (text.hasData())
 				{
-					text.copyTo(userData.middle_name, sizeof(userData.middle_name));
+					text.copyTo(userData->middle_name, sizeof(userData->middle_name));
-					userData.middle_name_entered = true;
+					userData->middle_name_entered = true;
 				}
 				else
 				{
-					userData.middle_name_entered = false;
+					userData->middle_name_entered = false;
-					userData.middle_name_specified = true;
+					userData->middle_name_specified = true;
 				}
 				break;
 
 			case isc_dyn_user_last:
 				if (text.hasData())
 				{
-					text.copyTo(userData.last_name, sizeof(userData.last_name));
+					text.copyTo(userData->last_name, sizeof(userData->last_name));
-					userData.last_name_entered = true;
+					userData->last_name_entered = true;
 				}
 				else
 				{
-					userData.last_name_entered = false;
+					userData->last_name_entered = false;
-					userData.last_name_specified = true;
+					userData->last_name_specified = true;
 				}
 				break;
 			}
 		}
 
-		int errcode = sec->execute(status, &userData);
+		USHORT id = tra->getUserManagement()->put(userData);
-
+		DeferredWork* work = DFW_post_work(tra, dfw_user_management, NULL, id);
-		switch (errcode)
-		{
-		case 0: // nothing
-		    break;
-		case GsecMsg22:
-			status_exception::raise(Arg::Gds(ENCODE_ISC_MSG(errcode, GSEC_MSG_FAC)) <<
-									Arg::Str(userData.user_name));
-		default:
-			status_exception::raise(Arg::Gds(ENCODE_ISC_MSG(errcode, GSEC_MSG_FAC)));
-		}
 	}
 	catch (const Exception& e)
 	{

src/jrd/tra.cpp

@@ -3472,6 +3472,7 @@ jrd_tra::~jrd_tra()
 {
 	delete tra_undo_record;
 	delete tra_undo_space;
+	delete tra_user_management;
 
 	if (!tra_outer)
 	{

src/jrd/tra.h

@@ -60,6 +60,8 @@ class ArrayField;
 class Attachment;
 class DeferredWork;
 class dsql_opn;
+class UserManagement;
+class thread_db;
 
 // Blobs active in transaction identified by bli_temp_id. Please keep this
 // structure small as there can be huge amount of them floating in memory.
@@ -120,7 +122,8 @@ public:
 		tra_transactions(*p),
 		tra_blob_space(NULL),
 		tra_undo_space(NULL),
-		tra_undo_record(NULL)
+		tra_undo_record(NULL),
+		tra_user_management(NULL)
 	{
 		if (outer)
 		{
@@ -212,6 +215,7 @@ private:
 	TempSpace* tra_undo_space;	// undo log storage
 
 	Record* tra_undo_record;	// temporary record used for the undo purposes
+	UserManagement* tra_user_management;
 
 public:
 	SSHORT getLockWait() const
@@ -250,6 +254,9 @@ public:
 
 		return tra_undo_record;
 	}
+
+	// get UserManagement object for transaction, defined in UserManagement.cpp
+	UserManagement* getUserManagement();
 };
 
 // System transaction is always transaction 0.
@@ -327,7 +334,7 @@ public:
 /* Savepoint block flags. */
 
 const int SAV_trans_level	= 1;	/* savepoint was started by TRA_start */
-const int SAV_event_post	= 2;	/* event posted in the save point */
+const int SAV_force_dfw		= 2;	/* DFW is present even if savepoint is empty */
 const int SAV_user			= 4;	/* named user savepoint as opposed to system ones */
 
 /* Maximum size in bytes of transaction-level savepoint data.

src/jrd/vio.cpp

@@ -2909,7 +2909,7 @@ void VIO_verb_cleanup(thread_db* tdbb, jrd_tra* transaction)
 
 /* Cleanup/merge deferred work/event post */
 
-	if (sav_point->sav_verb_actions || (sav_point->sav_flags & SAV_event_post))
+	if (sav_point->sav_verb_actions || (sav_point->sav_flags & SAV_force_dfw))
 	{
 		if (sav_point->sav_verb_count) {
 			DFW_delete_deferred(transaction, sav_point->sav_number);
@@ -2923,13 +2923,13 @@ void VIO_verb_cleanup(thread_db* tdbb, jrd_tra* transaction)
 		 * not rolled back, set flag for the previous save point.
 		 */
 
-		if (sav_point->sav_flags & SAV_event_post) {
+		if (sav_point->sav_flags & SAV_force_dfw) {
 			if (transaction->tra_save_point && !sav_point->sav_verb_count)
 			{
-				transaction->tra_save_point->sav_flags |= SAV_event_post;
+				transaction->tra_save_point->sav_flags |= SAV_force_dfw;
 			}
 
-			sav_point->sav_flags &= ~SAV_event_post;
+			sav_point->sav_flags &= ~SAV_force_dfw;
 		}
 	}