Refactor module so Cleanup routine can use private functions as need

Move completed job data receiving into private function give Receive-RSJob a chance to receive data (there was race condition possible) Implemented proxb#193
MVKozlov · Dec 27, 2018 · 01934d8 · 01934d8
1 parent 40020a5
commit 01934d8
Show file tree

Hide file tree

Showing 5 changed files with 123 additions and 84 deletions.
diff --git a/PoshRSJob/PoshRSJob.psd1 b/PoshRSJob/PoshRSJob.psd1
@@ -88,7 +88,7 @@ FileList = 'PoshRSJob.psd1', 'PoshRSJob.psm1', 'en-US\about_PoshRSJob.help.txt',
 'Private\FindFunction.ps1', 'Private\GetFunctionByFile.ps1', 'Private\GetFunctionDefinitionByFunction.ps1', 'Private\GetParamVariable.ps1', 'Private\GetUsingVariables.ps1', 'Private\GetUsingVariablesV2.ps1',
 'Private\Increment.ps1', 'Private\RegisterScriptScopeFunction.ps1', 'Public\Get-RSJob.ps1', 'Public\Receive-RSJob.ps1',
 'Public\Remove-RSJob.ps1', 'Public\Start-RSJob.ps1', 'Public\Stop-RSJob.ps1', 'Public\Wait-RSJob.ps1', 'TypeData\PoshRSJob.Format.ps1xml', 'TypeData\PoshRSJob.Types.ps1xml',
-'Private\SetIsReceived.ps1'
+'Private\SetIsReceived.ps1', 'Private\TryReceiveData.ps1'
 
 # Private data to pass to the module specified in ModuleToProcess
 PrivateData = @{

diff --git a/PoshRSJob/PoshRSJob.psm1 b/PoshRSJob/PoshRSJob.psm1
@@ -133,6 +133,41 @@ Else {
 "@
 }
 
+#region Load Public Functions
+Try {
+    Get-ChildItem "$ScriptPath\Public" -Filter *.ps1 | Select-Object -ExpandProperty FullName | ForEach-Object {
+        $Function = Split-Path $_ -Leaf
+        . $_
+    }
+} Catch {
+    Write-Warning ("{0}: {1}" -f $Function,$_.Exception.Message)
+    Continue
+}
+#endregion Load Public Functions
+
+#region Load Private Functions
+Try {
+    Get-ChildItem "$ScriptPath\Private" -Filter *.ps1 | Select-Object -ExpandProperty FullName | ForEach-Object {
+        $Function = Split-Path $_ -Leaf
+        . $_
+    }
+} Catch {
+    Write-Warning ("{0}: {1}" -f $Function,$_.Exception.Message)
+    Continue
+}
+#endregion Load Private Functions
+
+#region Format and Type Data
+Try {
+    Update-FormatData "$ScriptPath\TypeData\PoshRSJob.Format.ps1xml" -ErrorAction Stop
+}
+Catch {}
+Try {
+    Update-TypeData "$ScriptPath\TypeData\PoshRSJob.Types.ps1xml" -ErrorAction Stop
+}
+Catch {}
+#endregion Format and Type Data
+
 #region RSJob Variables
 Write-Verbose "Creating RS collections"
 New-Variable PoshRS_Jobs -Value ([System.Collections.ArrayList]::Synchronized([System.Collections.ArrayList]@())) -Option ReadOnly -Scope Global -Force
@@ -147,7 +182,13 @@ Write-Verbose "Creating routine to monitor RS jobs"
 $PoshRS_jobCleanup.Flag=$True
 $PoshRS_jobCleanup.Host = $Host
 $PSModulePath = $env:PSModulePath
-$PoshRS_jobCleanup.Runspace =[runspacefactory]::CreateRunspace()
+
+$TryReceiveDataDefinition = Get-Content Function:\TryReceiveData -ErrorAction Stop
+$SS_TryReceiveDataFunction = New-Object System.Management.Automation.Runspaces.SessionStateFunctionEntry -ArgumentList 'TryReceiveData', $TryReceiveDataDefinition
+$PoshRS_jobCleanup_InitialSessionState = [System.Management.Automation.Runspaces.InitialSessionState]::CreateDefault()
+$PoshRS_jobCleanup_InitialSessionState.Commands.Add($SS_TryReceiveDataFunction)
+
+$PoshRS_jobCleanup.Runspace =[runspacefactory]::CreateRunspace($PoshRS_jobCleanup_InitialSessionState)
 $PoshRS_jobCleanup.Runspace.Open()
 $PoshRS_jobCleanup.Runspace.SessionStateProxy.SetVariable("PoshRS_jobCleanup",$PoshRS_jobCleanup)
 $PoshRS_jobCleanup.Runspace.SessionStateProxy.SetVariable("PoshRS_Jobs",$PoshRS_Jobs)
@@ -159,53 +200,13 @@ $PoshRS_jobCleanup.PowerShell = [PowerShell]::Create().AddScript({
         try {
             Foreach($job in $PoshRS_Jobs) {
                 if ($job.RunDate -eq $null) {
-                    # ScriptProperty defined in PoshRSJob.Types.ps1xml doesn't work here
-                    # (why ??? It doesn't work even when I move Update-TypeData above of this code)
-                    # so $job.State always contains NotStarted and I need to get current state by method
-                    $State = $job.GetState()
-                    if ($State -ne [System.Management.Automation.PSInvocationState]::NotStarted) {
+                    # After moving all function import and PoshRSJob.Types.ps1xml above it now work as property
+                    #$State = $job.GetState()
+                    if ($job.State -ne [System.Management.Automation.PSInvocationState]::NotStarted) {
                         $job.RunDate = Get-Date
                     }
                 }
-
-                If ($job.Handle.isCompleted -AND (-NOT $Job.Completed)) {
-                    #$PoshRS_jobCleanup.Host.UI.WriteVerboseLine("$($Job.Id) completed")
-                    $Data = $null
-                    $CaughtErrors = $null
-                    Try {
-                        $Data = $job.InnerJob.EndInvoke($job.Handle)
-                    } Catch {
-                        $CaughtErrors = $Error
-                        #$PoshRS_jobCleanup.Host.UI.WriteVerboseLine("$($Job.Id) Caught terminating Error in job: $_")
-                    }
-                    #$PoshRS_jobCleanup.Host.UI.WriteVerboseLine("$($Job.Id) Checking for errors")
-                    If ($job.InnerJob.Streams.Error -OR $CaughtErrors) {
-                        #$PoshRS_jobCleanup.Host.UI.WriteVerboseLine("$($Job.Id) Errors Found!")
-                        $ErrorList = New-Object System.Management.Automation.PSDataCollection[System.Management.Automation.ErrorRecord]
-                        If ($job.InnerJob.Streams.Error) {
-                            ForEach ($Err in $job.InnerJob.Streams.Error) {
-                                #$PoshRS_jobCleanup.Host.UI.WriteVerboseLine("`t$($Job.Id) Adding Error")
-                                [void]$ErrorList.Add($Err)
-                            }
-                        }
-                        If ($CaughtErrors) {
-                            ForEach ($Err in $CaughtErrors) {
-                                #$PoshRS_jobCleanup.Host.UI.WriteVerboseLine("`t$($Job.Id) Adding Error")
-                                [void]$ErrorList.Add($Err)
-                            }
-                        }
-                        $job.Error = $ErrorList
-                    }
-                    #$PoshRS_jobCleanup.Host.UI.WriteVerboseLine("$($Job.Id) Disposing job")
-                    $job.InnerJob.dispose()
-                    #Return type from Invoke() is a generic collection; need to verify the first index is not NULL
-                    If ($Data -and ($Data.Count -gt 0) -AND (-NOT ($Data.Count -eq 1 -AND $Null -eq $Data[0]))) {
-                        $job.output = $Data
-                        $job.HasMoreData = $True
-                    }
-                    $Error.Clear()
-                    $job.Completed = $True
-                }
+                TryReceiveData $Job
             }
         }
         finally {
@@ -290,41 +291,6 @@ $PoshRS_RunspacePoolCleanup.PowerShell.Runspace = $PoshRS_RunspacePoolCleanup.Ru
 $PoshRS_RunspacePoolCleanup.Handle = $PoshRS_RunspacePoolCleanup.PowerShell.BeginInvoke()
 #endregion Cleanup Routine
 
-#region Load Public Functions
-Try {
-    Get-ChildItem "$ScriptPath\Public" -Filter *.ps1 | Select-Object -ExpandProperty FullName | ForEach-Object {
-        $Function = Split-Path $_ -Leaf
-        . $_
-    }
-} Catch {
-    Write-Warning ("{0}: {1}" -f $Function,$_.Exception.Message)
-    Continue
-}
-#endregion Load Public Functions
-
-#region Load Private Functions
-Try {
-    Get-ChildItem "$ScriptPath\Private" -Filter *.ps1 | Select-Object -ExpandProperty FullName | ForEach-Object {
-        $Function = Split-Path $_ -Leaf
-        . $_
-    }
-} Catch {
-    Write-Warning ("{0}: {1}" -f $Function,$_.Exception.Message)
-    Continue
-}
-#endregion Load Private Functions
-
-#region Format and Type Data
-Try {
-    Update-FormatData "$ScriptPath\TypeData\PoshRSJob.Format.ps1xml" -ErrorAction Stop
-}
-Catch {}
-Try {
-    Update-TypeData "$ScriptPath\TypeData\PoshRSJob.Types.ps1xml" -ErrorAction Stop
-}
-Catch {}
-#endregion Format and Type Data
-
 #region Aliases
 New-Alias -Name ssj -Value Start-RSJob -Force
 New-Alias -Name gsj -Value Get-RSJob -Force

diff --git a/PoshRSJob/Private/TryReceiveData.ps1 b/PoshRSJob/Private/TryReceiveData.ps1
@@ -0,0 +1,49 @@
+function TryReceiveData {
+param(
+    [Parameter(ValueFromPipeline = $true)]
+    [RSJob]$Job
+)
+PROCESS {
+    If ($Job.Handle.isCompleted -AND (-NOT $Job.Completed)) {
+        #$PoshRS_jobCleanup.Host.UI.WriteVerboseLine("$($Job.Id) completed")
+        $Data = $null
+        $CaughtErrors = $null
+        Try {
+            $Data = $Job.InnerJob.EndInvoke($Job.Handle)
+        } Catch {
+            $CaughtErrors = $_
+            #$PoshRS_jobCleanup.Host.UI.WriteVerboseLine("$($Job.Id) Caught terminating Error in job: $_")
+        }
+        #$PoshRS_jobCleanup.Host.UI.WriteVerboseLine("$($Job.Id) Checking for errors ($($Job.InnerJob.Streams.Error.Count)) & ($($null -ne $CaughtErrors))")
+        If ($Job.InnerJob.Streams.Error.Count -ne 0 -or $null -ne $CaughtErrors) {
+            #$PoshRS_jobCleanup.Host.UI.WriteVerboseLine("$($Job.Id) Errors Found!")
+            $ErrorList = New-Object System.Management.Automation.PSDataCollection[System.Management.Automation.ErrorRecord]
+            If ($Job.InnerJob.Streams.Error) {
+                ForEach ($Err in $Job.InnerJob.Streams.Error) {
+                    #$PoshRS_jobCleanup.Host.UI.WriteVerboseLine("`t$($Job.Id) Adding Error")
+                    [void]$ErrorList.Add($Err)
+                }
+            }
+            If ($null -ne $CaughtErrors) {
+                ForEach ($Err in $CaughtErrors) {
+                    #$PoshRS_jobCleanup.Host.UI.WriteVerboseLine("`t$($Job.Id) Adding Error")
+                    [void]$ErrorList.Add($Err)
+                }
+            }
+            #$PoshRS_jobCleanup.Host.UI.WriteVerboseLine("$($Job.Id) $($ErrorList.Count) Errors Found!")
+            $Job.Error = $ErrorList
+        }
+        #$PoshRS_jobCleanup.Host.UI.WriteVerboseLine("$($Job.Id) Disposing job")
+        $Job.InnerJob.dispose()
+        #Return type from Invoke() is a generic collection; need to verify the first index is not NULL
+        If ($Data -and ($Data.Count -gt 0) -AND (-NOT ($Data.Count -eq 1 -AND $Null -eq $Data[0]))) {
+            $Job.output = $Data
+            #It's not needed because HasMoreData is a ScriptProperty
+            #$Job.HasMoreData = $True
+        }
+        #$Error.Clear()
+        $Job.Completed = $True
+    }
+}
+
+}
diff --git a/PoshRSJob/Public/Receive-RSJob.ps1 b/PoshRSJob/Public/Receive-RSJob.ps1
@@ -81,6 +81,13 @@ Function Receive-RSJob {
         # Will be good to obsolete any other parameters except Job
         if ($Property -eq 'Job') { # Receive data right from pipeline
             if ($Job) {
+                [System.Threading.Monitor]::Enter($PoshRS_Jobs.syncroot)
+                try {
+                    $Job | TryReceiveData
+                }
+                finally {
+                    [System.Threading.Monitor]::Exit($PoshRS_Jobs.syncroot)
+                }
                 $Job | WriteStream
                 if ($isReseivedStates -contains $Job.State) {
                     $Job | SetIsReceived -SetTrue

diff --git a/PoshRSJob/Public/Stop-RSJob.ps1 b/PoshRSJob/Public/Stop-RSJob.ps1
@@ -82,9 +82,20 @@ Function Stop-RSJob {
                         Write-Verbose "Stopping $($_.InstanceId)"
                         if ($_.State -ne 'Completed' -and $_.State -ne 'Failed' -and $_.State -ne 'Stopped') {
                             Write-Verbose "Killing job $($_.InstanceId)"
-                            [void] $_.InnerJob.Stop()
+                            if ($PassThru) {
+                                [void] $_.InnerJob.Stop()
+                                $_
+                            }
+                            else {
+                                [void]$List.Add(
+                                    (New-Object -Typename PSObject -Property @{
+                                        Job = $_
+                                        StopHandle = $_.InnerJob.BeginStop($null, $null)
+                                    })
+                                )
+                            }
                         }
-                        if ($PassThru) {
+                        elseif ($PassThru) {
                             $_
                         }
                     }
@@ -101,7 +112,13 @@ Function Stop-RSJob {
         }
     }
     End {
-        if ($List.Count) { # obsolete parameter sets used
+        if ($PSCmdlet.ParameterSetName -eq 'Job' -and $List.Count -gt 0) {
+            Write-Debug "End"
+            foreach ($o in $List) {
+                $o.Job.InnerJob.EndStop($o.StopHandle)
+            }
+        }
+        elseif ($List.Count) { # obsolete parameter sets used
             $PSBoundParameters[$Property] = $List
             [void]$PSBoundParameters.Remove('PassThru')
             Get-RSJob @PSBoundParameters | Stop-RSJob -PassThru:$PassThru