From 65e49e2139d8a1c832d0828fa5d0d5937a628099 Mon Sep 17 00:00:00 2001
From: Eric Espie <eric.espie@combodo.com>
Date: Mon, 27 Jan 2025 17:34:46 +0100
Subject: [PATCH] =?UTF-8?q?N=C2=B08139=20-=20Avoid=20double=20writing=20in?=
 =?UTF-8?q?=20lifecycle=20action?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

---
 core/dbobject.class.php | 111 ++++++++++++++++++++--------------------
 1 file changed, 55 insertions(+), 56 deletions(-)

diff --git a/core/dbobject.class.php b/core/dbobject.class.php
index b08d3e9cd..869957265 100644
--- a/core/dbobject.class.php
+++ b/core/dbobject.class.php
@@ -3624,13 +3624,13 @@ abstract class DBObject implements iDisplay
 	 */
 	public function DBUpdate()
 	{
+		$this->LogCRUDEnter(__METHOD__);
 		if (!MetaModel::StartReentranceProtection($this)) {
 			$this->LogCRUDExit(__METHOD__, 'Rejected (reentrance)');
 
 			return false;
 		}
 
-		$this->LogCRUDEnter(__METHOD__);
 		if (!$this->m_bIsInDB)
 		{
 			throw new CoreException("DBUpdate: could not update a newly created object, please call DBInsert instead");
@@ -4545,7 +4545,6 @@ abstract class DBObject implements iDisplay
 
 		// Change the state before proceeding to the actions, this is necessary because an action might
 		// trigger another stimuli (alternative: push the stimuli into a queue)
-		$sPreviousState = $this->Get($sStateAttCode);
 		$sNewState = $aTransitionDef['target_state'];
 		$this->Set($sStateAttCode, $sNewState);
 
@@ -4553,64 +4552,64 @@ abstract class DBObject implements iDisplay
 		//    array('target_state'=>..., 'actions'=>array of handlers procs, 'user_restriction'=>TBD
 
 		$bSuccess = true;
-		$sActionDesc = '';
-		foreach ($aTransitionDef['actions'] as $actionHandler)
-		{
-			if (is_string($actionHandler))
-			{
-				// Old (pre-2.1.0 modules) action definition without any parameter
-				$aActionCallSpec = array($this, $actionHandler);
-				$sActionDesc = $sClass.'::'.$actionHandler;
+		// Prevent current object from being updated by the actions
+		$this->AddCurrentObjectInCrudStack('APPLY_STIMULUS');
+		MetaModel::StartReentranceProtection($this);
+		try {
+			foreach ($aTransitionDef['actions'] as $actionHandler) {
+				if (is_string($actionHandler)) {
+					// Old (pre-2.1.0 modules) action definition without any parameter
+					$aActionCallSpec = array($this, $actionHandler);
+					$sActionDesc = $sClass.'::'.$actionHandler;
 
-				if (!is_callable($aActionCallSpec))
-				{
-					throw new CoreException("Unable to call action: $sClass::$actionHandler");
-				}
-				$bRet = call_user_func($aActionCallSpec, $sStimulusCode);
-			}
-			else // if (is_array($actionHandler))
-			{
-				// New syntax: 'verb' and typed parameters
-				$sAction = $actionHandler['verb'];
-				$sActionDesc = "$sClass::$sAction";
-				$aParams = array();
-				foreach($actionHandler['params'] as $aDefinition)
-				{
-					$sParamType = array_key_exists('type', $aDefinition) ? $aDefinition['type'] : 'string';
-					switch($sParamType)
-					{
-						case 'int':
-							$value = (int)$aDefinition['value'];
-							break;
-
-						case 'float':
-							$value = (float)$aDefinition['value'];
-							break;
-
-						case 'bool':
-							$value = (bool)$aDefinition['value'];
-							break;
-
-						case 'reference':
-							$value = ${$aDefinition['value']};
-							break;
-
-						case 'string':
-						default:
-							$value = (string)$aDefinition['value'];
+					if (!is_callable($aActionCallSpec)) {
+						throw new CoreException("Unable to call action: $sClass::$actionHandler");
 					}
-					$aParams[] = $value;
+					$bRet = call_user_func($aActionCallSpec, $sStimulusCode);
+				} else // if (is_array($actionHandler))
+				{
+					// New syntax: 'verb' and typed parameters
+					$sAction = $actionHandler['verb'];
+					$sActionDesc = "$sClass::$sAction";
+					$aParams = array();
+					foreach ($actionHandler['params'] as $aDefinition) {
+						$sParamType = array_key_exists('type', $aDefinition) ? $aDefinition['type'] : 'string';
+						switch ($sParamType) {
+							case 'int':
+								$value = (int)$aDefinition['value'];
+								break;
+
+							case 'float':
+								$value = (float)$aDefinition['value'];
+								break;
+
+							case 'bool':
+								$value = (bool)$aDefinition['value'];
+								break;
+
+							case 'reference':
+								$value = ${$aDefinition['value']};
+								break;
+
+							case 'string':
+							default:
+								$value = (string)$aDefinition['value'];
+						}
+						$aParams[] = $value;
+					}
+					$aCallSpec = array($this, $sAction);
+					$bRet = call_user_func_array($aCallSpec, $aParams);
+				}
+				// if one call fails, the whole is considered as failed
+				// (in case there is no returned value, null is obtained and means "ok")
+				if ($bRet === false) {
+					IssueLog::Info("Lifecycle action $sActionDesc returned false on object #$sClass:".$this->GetKey());
+					$bSuccess = false;
 				}
-				$aCallSpec = array($this, $sAction);
-				$bRet = call_user_func_array($aCallSpec, $aParams);
-			}
-			// if one call fails, the whole is considered as failed
-			// (in case there is no returned value, null is obtained and means "ok")
-			if ($bRet === false)
-			{
-				IssueLog::Info("Lifecycle action $sActionDesc returned false on object #$sClass:".$this->GetKey());
-				$bSuccess = false;
 			}
+		} finally {
+			MetaModel::StopReentranceProtection($this);
+			$this->RemoveCurrentObjectInCrudStack();
 		}
 		if ($bSuccess)
 		{