Bugfix for CORE-5275: Expression index may become inconsistent if CREATE

INDEX was interrupted after b-tree creation but before commiting.

src/jrd/dfw.epp

@@ -494,6 +494,7 @@ static void check_computed_dependencies(thread_db* tdbb, jrd_tra* transaction,
 	const Firebird::MetaName& fieldName);
 static void check_dependencies(thread_db*, const TEXT*, const TEXT*, const TEXT*, int, jrd_tra*);
 static void check_filename(const Firebird::string&, bool);
+static void cleanup_index_creation(thread_db*, DeferredWork*, jrd_tra*);
 static bool formatsAreEqual(const Format*, const Format*);
 static bool	find_depend_in_dfw(thread_db*, TEXT*, USHORT, USHORT, jrd_tra*);
 static void get_array_desc(thread_db*, const TEXT*, Ods::InternalArrayDesc*);
@@ -1433,10 +1434,13 @@ void DFW_perform_work(thread_db* tdbb, jrd_tra* transaction)
 	{
 		more = false;
 		try {
-			tdbb->tdbb_flags |= (TDBB_dont_post_dfw | TDBB_use_db_page_space);
+			tdbb->tdbb_flags |= (TDBB_dont_post_dfw | TDBB_use_db_page_space |
+				(phase == 0 ? TDBB_dfw_cleanup : 0));
+
 			for (const deferred_task* task = task_table; task->task_type != dfw_null; ++task)
 			{
-				for (DeferredWork* work = transaction->tra_deferred_job->work; work; work = work->getNext())
+				for (DeferredWork* work = transaction->tra_deferred_job->work;
+					work; work = work->getNext())
 				{
 					if (work->dfw_type == task->task_type)
 					{
@@ -1451,17 +1455,20 @@ void DFW_perform_work(thread_db* tdbb, jrd_tra* transaction)
 					}
 				}
 			}
+
+			tdbb->tdbb_flags &= ~(TDBB_dont_post_dfw | TDBB_use_db_page_space | TDBB_dfw_cleanup);
+
 			if (!phase)
 			{
 				fb_utils::copyStatus(tdbb->tdbb_status_vector, &err_status);
 				ERR_punt();
 			}
+
 			++phase;
-			tdbb->tdbb_flags &= ~(TDBB_dont_post_dfw | TDBB_use_db_page_space);
 		}
 		catch (const Firebird::Exception& ex)
 		{
-			tdbb->tdbb_flags &= ~(TDBB_dont_post_dfw | TDBB_use_db_page_space);
+			tdbb->tdbb_flags &= ~(TDBB_dont_post_dfw | TDBB_use_db_page_space | TDBB_dfw_cleanup);
 
 			// Do any necessary cleanup
 			if (!phase)
@@ -2603,6 +2610,7 @@ static bool create_expression_index(thread_db* tdbb, SSHORT phase, DeferredWork*
 	switch (phase)
 	{
 	case 0:
+		cleanup_index_creation(tdbb, work, transaction);
 		MET_delete_dependencies(tdbb, work->dfw_name, obj_expression_index, transaction);
 		return false;
 
@@ -2946,6 +2954,64 @@ static void check_filename(const Firebird::string& name, bool shareExpand)
 }
 
 
+static void cleanup_index_creation(thread_db* tdbb, DeferredWork* work, jrd_tra* transaction)
+{
+	Database* const dbb = tdbb->getDatabase();
+
+	AutoRequest request;
+
+	FOR(REQUEST_HANDLE request TRANSACTION_HANDLE transaction) IDXN IN RDB$INDICES CROSS
+		IREL IN RDB$RELATIONS OVER RDB$RELATION_NAME
+			WITH IDXN.RDB$INDEX_NAME EQ work->dfw_name.c_str()
+			// dimitr: I have no idea why the condition below is required here
+			AND IREL.RDB$VIEW_BLR MISSING // views do not have indices
+	{
+		jrd_rel* const relation = MET_lookup_relation(tdbb, IDXN.RDB$RELATION_NAME);
+		RelationPages* const relPages = relation->getPages(tdbb, MAX_TRA_NUMBER, false);
+
+		if (relPages && relPages->rel_index_root)
+		{
+			// We need to special handle temp tables with ON PRESERVE ROWS only
+			const bool isTempIndex = (relation->rel_flags & REL_temp_conn) &&
+				(relPages->rel_instance_id != 0);
+
+			// Fetch the root index page and mark MUST_WRITE, and then
+			// delete the index. It will also clean the index slot.
+
+			if (work->dfw_id != dbb->dbb_max_idx)
+			{
+				WIN window(relPages->rel_pg_space_id, relPages->rel_index_root);
+				CCH_FETCH(tdbb, &window, LCK_write, pag_root);
+				CCH_MARK_MUST_WRITE(tdbb, &window);
+				const bool tree_exists = BTR_delete_index(tdbb, &window, work->dfw_id);
+
+				if (!isTempIndex) {
+					work->dfw_id = dbb->dbb_max_idx;
+				}
+				else if (tree_exists)
+				{
+					IndexLock* const idx_lock = CMP_get_index_lock(tdbb, relation, work->dfw_id);
+
+					if (idx_lock)
+					{
+						if (!--idx_lock->idl_count)
+							LCK_release(tdbb, idx_lock->idl_lock);
+					}
+				}
+			}
+
+			if (!IDXN.RDB$INDEX_ID.NULL)
+			{
+				MODIFY IDXN USING
+					IDXN.RDB$INDEX_ID.NULL = TRUE;
+				END_MODIFY
+			}
+		}
+	}
+	END_FOR
+}
+
+
 static bool formatsAreEqual(const Format* old_format, const Format* new_format)
 {
 /**************************************
@@ -3147,7 +3213,6 @@ static bool create_index(thread_db* tdbb, SSHORT phase, DeferredWork* work, jrd_
 	jrd_rel* partner_relation;
 	index_desc idx;
 	int key_count;
-	AutoRequest handle;
 
 	SET_TDBB(tdbb);
 	Jrd::Attachment* attachment = tdbb->getAttachment();
@@ -3156,65 +3221,7 @@ static bool create_index(thread_db* tdbb, SSHORT phase, DeferredWork* work, jrd_
 	switch (phase)
 	{
 	case 0:
-		handle.reset();
+		cleanup_index_creation(tdbb, work, transaction);
-
-		// Drop those indices at clean up time.
-		FOR(REQUEST_HANDLE handle TRANSACTION_HANDLE transaction) IDXN IN RDB$INDICES CROSS
-			IREL IN RDB$RELATIONS OVER RDB$RELATION_NAME
-				WITH IDXN.RDB$INDEX_NAME EQ work->dfw_name.c_str()
-		{
-			// Views do not have indices
-			if (IREL.RDB$VIEW_BLR.NULL)
-			{
-				relation = MET_lookup_relation(tdbb, IDXN.RDB$RELATION_NAME);
-
-				RelationPages* relPages = relation->getPages(tdbb, MAX_TRA_NUMBER, false);
-				if (!relPages) {
-					return false;
-				}
-
-				// we need to special handle temp tables with ON PRESERVE ROWS only
-				const bool isTempIndex = (relation->rel_flags & REL_temp_conn) &&
-					(relPages->rel_instance_id != 0);
-
-				// Fetch the root index page and mark MUST_WRITE, and then
-				// delete the index. It will also clean the index slot. Note
-				// that the previous fixed definition of MAX_IDX (64) has been
-				// dropped in favor of a computed value saved in the Database
-				if (relPages->rel_index_root)
-				{
-					if (work->dfw_id != dbb->dbb_max_idx)
-					{
-						WIN window(relPages->rel_pg_space_id, relPages->rel_index_root);
-						CCH_FETCH(tdbb, &window, LCK_write, pag_root);
-						CCH_MARK_MUST_WRITE(tdbb, &window);
-						const bool tree_exists = BTR_delete_index(tdbb, &window, work->dfw_id);
-
-						if (!isTempIndex) {
-							work->dfw_id = dbb->dbb_max_idx;
-						}
-						else if (tree_exists)
-						{
-							IndexLock* idx_lock = CMP_get_index_lock(tdbb, relation, work->dfw_id);
-							if (idx_lock)
-							{
-								if (!--idx_lock->idl_count) {
-									LCK_release(tdbb, idx_lock->idl_lock);
-								}
-							}
-						}
-					}
-					if (!IDXN.RDB$INDEX_ID.NULL)
-					{
-						MODIFY IDXN USING
-							IDXN.RDB$INDEX_ID.NULL = TRUE;
-						END_MODIFY
-					}
-				}
-			}
-		}
-		END_FOR
-
 		return false;
 
 	case 1:

src/jrd/jrd.cpp

@@ -7426,7 +7426,7 @@ ISC_STATUS thread_db::checkCancelState()
 	// nor currently detaching, as these actions should never be interrupted.
 	// Also don't break wait in LM if it is not safe.
 
-	if (tdbb_flags & (TDBB_verb_cleanup | TDBB_detaching | TDBB_wait_cancel_disable))
+	if (tdbb_flags & (TDBB_verb_cleanup | TDBB_dfw_cleanup | TDBB_detaching | TDBB_wait_cancel_disable))
 		return FB_SUCCESS;
 
 	if (attachment)

src/jrd/jrd.h

@@ -334,6 +334,7 @@ const USHORT TDBB_wait_cancel_disable	= 1024;		// don't cancel current waiting o
 const USHORT TDBB_cache_unwound			= 2048;		// page cache was unwound
 const USHORT TDBB_trusted_ddl			= 4096;		// skip DDL permission checks. Set after DDL permission check and clear after DDL execution
 const USHORT TDBB_reset_stack			= 8192;		// stack should be reset after stack overflow exception
+const USHORT TDBB_dfw_cleanup			= 16384;	// DFW cleanup phase is active
 
 class thread_db : public Firebird::ThreadData
 {