From 478042dc67c47040917b3d487bed9e1f9a8cda8f Mon Sep 17 00:00:00 2001 From: isc-tleavitt <73311181+isc-tleavitt@users.noreply.github.com> Date: Wed, 11 Dec 2024 10:47:31 -0500 Subject: [PATCH 1/3] fix: don't add needless mappings This was leading to deadlock in perfectly reasonable cases and still could lead to deadlock in less-reasonable cases. --- CHANGELOG.md | 1 + .../IPM/ResourceProcessor/Default/Class.cls | 2 +- .../ResourceProcessor/Default/Document.cls | 29 +++++++++---------- .../Default/LocalizedMessages.cls | 2 +- .../IPM/ResourceProcessor/Default/Package.cls | 4 +-- .../IPM/ResourceProcessor/Default/Routine.cls | 2 +- 6 files changed, 19 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 076c3431..541c98fa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -60,6 +60,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - #559: Allow treating the "w" in SemVer x.y.z-w as a post-release rather than pre-release. - #607: Uninstall reports deletion of non-classes - #606: Don't put garbage folders in tar archive +- #652: Don't create extra needless mappings (could cause deadlock on multithreaded installation) ### Security - diff --git a/src/cls/IPM/ResourceProcessor/Default/Class.cls b/src/cls/IPM/ResourceProcessor/Default/Class.cls index 08f7f3a4..2255d355 100644 --- a/src/cls/IPM/ResourceProcessor/Default/Class.cls +++ b/src/cls/IPM/ResourceProcessor/Default/Class.cls @@ -19,7 +19,7 @@ Method OnConfigureMappings(ByRef pParams) As %Status Set tName = $Piece(..ResourceReference.Name,".",1,*-1) Set tGlobalScope = ..ResourceReference.Module.GlobalScope && '$Get(pParams("Reload","ForceLocalScope"),0) If 'tGlobalScope { - Set tNeedExplicitMapping = ..FileExistsInCurrentNS(tName) + Set tNeedExplicitMapping = ..ResourceIsMappedToDefaultDB(..ResourceReference.Name) If tNeedExplicitMapping { Set tPackage = $p(tName,".",1,*-1) Set tSourceDB = ##class(%IPM.Utils.Module).GetRoutineDatabase($Namespace) diff --git a/src/cls/IPM/ResourceProcessor/Default/Document.cls b/src/cls/IPM/ResourceProcessor/Default/Document.cls index 8c160553..47c19c26 100644 --- a/src/cls/IPM/ResourceProcessor/Default/Document.cls +++ b/src/cls/IPM/ResourceProcessor/Default/Document.cls @@ -609,23 +609,20 @@ Method OnGetUniqueName(Output pUniqueName) /// Returns a boolean of whether current file exists in the the routine database of the current namespace exists in current namespace's database /// Helper method to be used in derived classes' OnConfigureMappings() to skip creating unnecessary mapping -ClassMethod FileExistsInCurrentNS(pFileName As %String) As %Boolean +ClassMethod ResourceIsMappedToDefaultDB(pResourceName As %String) As %Boolean { - Set tSourceDB = ##class(%IPM.Utils.Module).GetRoutineDatabase($Namespace) - // Check if current file exists in the the routine database of the current namespace exists in current namespace('s database) - // If yes, skip creating unnecessary mappings - // (Spec,Dir=1,OrderBy=1,SystemFiles=1,Flat,NotStudio=0,ShowGenerated=0,Filter,RoundTime=0,Mapped=0) - // Set mapped=0 since we only want to check whether the file exists in the current routine DB - Set tResult = ##class(%Library.RoutineMgr).StudioOpenDialogFunc(pFileName_".*",1,1,1,,,,,,0) - Set tNeedExplicitMapping = 1 - While tResult.%Next(.tSC) { - $$$ThrowOnError(tSC) - if ($ZConvert(tResult.%Get("Name"),"U") = $ZConvert(pFileName, "U")) { - Set tNeedExplicitMapping = 0 - } - } - $$$ThrowOnError(tSC) - Return tNeedExplicitMapping + Set defaultDB = ##class(%IPM.Utils.Module).GetRoutineDatabaseDir($Namespace) + Set name = $Piece(pResourceName,".",1,*-1) + Set type = $ZConvert($Piece(pResourceName,".",*),"U") + If (type = "PKG") { + Set db = ##class(%SYS.Namespace).GetPackageDest(,name) + } ElseIf (type = "CLS") { + Set db = ##class(%SYS.Namespace).GetPackageDest(,$Piece(name,".",1,*-1)) + } Else { + Set db = ##class(%SYS.Namespace).GetRoutineDest(,name) + } + Set db = $Piece(db,"^",2,*) // Slight difference in format of reporting here + Return (db = defaultDB) } } diff --git a/src/cls/IPM/ResourceProcessor/Default/LocalizedMessages.cls b/src/cls/IPM/ResourceProcessor/Default/LocalizedMessages.cls index f49ceee3..426d3ef8 100644 --- a/src/cls/IPM/ResourceProcessor/Default/LocalizedMessages.cls +++ b/src/cls/IPM/ResourceProcessor/Default/LocalizedMessages.cls @@ -55,7 +55,7 @@ Method OnConfigureMappings(ByRef pParams) As %Status Set tName = $Piece(..ResourceReference.Name,".",1,*-1) Set tGlobalScope = ..ResourceReference.Module.GlobalScope && '$Get(pParams("Reload","ForceLocalScope"),0) If 'tGlobalScope { - Set tNeedExplicitMapping = ..FileExistsInCurrentNS(tName) + Set tNeedExplicitMapping = ..ResourceIsMappedToDefaultDB(tName_".INC") If tNeedExplicitMapping { Set tSourceDB = ##class(%IPM.Utils.Module).GetRoutineDatabase($Namespace) Set tSC = ##class(%IPM.Utils.Module).AddRoutineMapping($namespace,tName,,tSourceDB) diff --git a/src/cls/IPM/ResourceProcessor/Default/Package.cls b/src/cls/IPM/ResourceProcessor/Default/Package.cls index 16c876df..10247b5e 100644 --- a/src/cls/IPM/ResourceProcessor/Default/Package.cls +++ b/src/cls/IPM/ResourceProcessor/Default/Package.cls @@ -16,7 +16,7 @@ Property Directory As %String(MAXLEN = "") [ InitialExpression = "cls" ]; Property LoadAsDirectory As %Boolean [ InitialExpression = 1 ]; /// Extension for individual filename(s) that comprise this resource -Property FilenameExtension As %String [ InitialExpression = "" ]; +Property FilenameExtension As %String; /// Subclasses may override to customize mapping behavior at the beginning of the Reload phase. Method OnConfigureMappings(ByRef pParams) As %Status @@ -26,7 +26,7 @@ Method OnConfigureMappings(ByRef pParams) As %Status Set tName = $Piece(..ResourceReference.Name,".",1,*-1) Set tGlobalScope = ..ResourceReference.Module.GlobalScope && '$Get(pParams("Reload","ForceLocalScope"),0) If 'tGlobalScope { - Set tNeedExplicitMapping = ..FileExistsInCurrentNS(tName) + Set tNeedExplicitMapping = ..ResourceIsMappedToDefaultDB(..ResourceReference.Name) If tNeedExplicitMapping { Set tSourceDB = ##class(%IPM.Utils.Module).GetRoutineDatabase($Namespace) Set tSC = ##class(%IPM.Utils.Module).AddPackageMapping($namespace,tName,tSourceDB) diff --git a/src/cls/IPM/ResourceProcessor/Default/Routine.cls b/src/cls/IPM/ResourceProcessor/Default/Routine.cls index 9b32e5fb..f051a7c0 100644 --- a/src/cls/IPM/ResourceProcessor/Default/Routine.cls +++ b/src/cls/IPM/ResourceProcessor/Default/Routine.cls @@ -22,7 +22,7 @@ Method OnConfigureMappings(ByRef pParams) As %Status Set tName = $Piece(..ResourceReference.Name,".",1,*-1) Set tGlobalScope = ..ResourceReference.Module.GlobalScope && '$Get(pParams("Reload","ForceLocalScope"),0) If 'tGlobalScope && '..LoadAsDirectory { - Set tNeedExplicitMapping = ..FileExistsInCurrentNS(tName) + Set tNeedExplicitMapping = ..ResourceIsMappedToDefaultDB(..ResourceReference.Name) If tNeedExplicitMapping { Set tSourceDB = ##class(%IPM.Utils.Module).GetRoutineDatabase($Namespace) Set tSC = ##class(%IPM.Utils.Module).AddRoutineMapping($namespace,tName,,tSourceDB) From 43e99574332eb3a1e225fd1f7202a37be278c014 Mon Sep 17 00:00:00 2001 From: isc-tleavitt <73311181+isc-tleavitt@users.noreply.github.com> Date: Wed, 11 Dec 2024 10:50:45 -0500 Subject: [PATCH 2/3] chore (docs): tweak changelog this is just to force CI to run... --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 541c98fa..ec7a425d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -60,7 +60,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - #559: Allow treating the "w" in SemVer x.y.z-w as a post-release rather than pre-release. - #607: Uninstall reports deletion of non-classes - #606: Don't put garbage folders in tar archive -- #652: Don't create extra needless mappings (could cause deadlock on multithreaded installation) +- #652: Don't create extra needless mappings (could cause deadlock with parallel installation of dependencies) ### Security - From 4f771561a2ae94ec15a65b7cebad4b779f0571e7 Mon Sep 17 00:00:00 2001 From: isc-tleavitt <73311181+isc-tleavitt@users.noreply.github.com> Date: Wed, 11 Dec 2024 11:04:06 -0500 Subject: [PATCH 3/3] chore (docs): update method description --- src/cls/IPM/ResourceProcessor/Default/Document.cls | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/cls/IPM/ResourceProcessor/Default/Document.cls b/src/cls/IPM/ResourceProcessor/Default/Document.cls index 47c19c26..533b0daa 100644 --- a/src/cls/IPM/ResourceProcessor/Default/Document.cls +++ b/src/cls/IPM/ResourceProcessor/Default/Document.cls @@ -607,8 +607,8 @@ Method OnGetUniqueName(Output pUniqueName) } } -/// Returns a boolean of whether current file exists in the the routine database of the current namespace exists in current namespace's database -/// Helper method to be used in derived classes' OnConfigureMappings() to skip creating unnecessary mapping +/// Helper method to be used in derived classes' OnConfigureMappings() to skip creating unnecessary mappings. +/// Returns true if pResourceName (in InternalName format - e.g., %Foo.Bar.PKG) is mapped to the current namespace's default routine database. ClassMethod ResourceIsMappedToDefaultDB(pResourceName As %String) As %Boolean { Set defaultDB = ##class(%IPM.Utils.Module).GetRoutineDatabaseDir($Namespace)