FEATURE REQUEST: make Chocolatey multiple instance friendly

[bcurran3]

RE: https://groups.google.com/forum/#!topic/chocolatey/GRJrv99RwK8

What You Are Seeing?

Attempting to run multiple concurrent instances of choco.exe can cause choco to become "confused."

In the past when I've attempted to run Chocolatey commands in two different administrative consoles on the same computer, I've bumped into problems. Not always, but often. Depends on what I'm trying to do. Not certain, but pretty sure it's always a Chocolatey licensed edition (I run Pro) package synchronization related issue. Though I encounter this issue specifically due to the licensed edition package synchronization feature, I believe this request would benefit Chocolatey globally in all editions.

What is Expected?

For choco.exe to check for another instance of itself and pause so no unexpected/unwanted behavior occurs. choco.exe could display a message such as "Pausing, another instance of choco.exe is running." or "Pausing, waiting for another instance of choco.exe to finish." then loop until the other instance is finished. A default timeout such as 5 minutes could be built in or a new configurable feature created and used to abort.

How Did You Get This To Happen? (Steps to Reproduce)

Here's a real world test done 5 minutes ago:

I opened up two Command Prompts with administrative rights. I arranged them so I could easily see them and access them fast. I typed "cinst atom" in one and "cinst git" in the other. I hit enter on both within a half second of each other. Both start installing. Atom finishes first while git takes longer being a meta package and having to download more packages. Atom finishes. While git.install is still installing, I attempt a "choco uninstall atom," subsequently many warnings ensue.

[mwallner]

this looks somehow similar to #1258

Idea 1: lock execution of choco.exe via a global mutex: although "blocking" choco.exe to run until another, currently running instance exists would be possible in theory I don't like the idea of a choco.exe that's doing a long running install (such as VS2017) blocking all other choco calls.

Idea 2: lock choco install only - better than idea 1, still a more sophisticated mechanism should be used (i.e. in addition to check for a named object, also check if another choco.exe is still running - otherwise the process may have been termiated externally and no other choco install would ever get the chance to run)

[bcurran3]

For anyone interested in choco being multiple instance friendly...

I've created this helper until such time:
https://chocolatey.org/packages/chocolatey-preinstaller-checks.extension

It's currently PRE-release. I have a little more work to do. Feedback welcome.

[bcurran3]

Just an update:
https://chocolatey.org/packages/chocolatey-preinstaller-checks.extension/0.0.1 works very well and extensively tested, I recently installed 100+ packages with it installed. Unfortunately it's still stuck in moderation purgatory until such time as @ferventcoder has time to moderate and disect it fully. Shame as it's a REALLY good extension that activates without needing any package modifications and completely stops choco.exe from stomping on itself and corrupting package data.

0.0.2-pre .nupkg is available on my GitHub repository:
https://github.com/bcurran3/ChocolateyPackages/tree/master/chocolatey-preinstaller-checks.extension
###CHANGE LOG:

• 0.0.2 - Added configurable options to only warn or pause and retry when multiple instances are found. Added configurable pause time for each of the multiple instance checks. Added conditional handling options for each of the three checks, i.e. option to abort install based on condition.
• 0.0.1 - initial release

Better explanation of 0.0.2 from the config

`<?xml version="1.0"?>
<Settings>
  <global>
    <AbortOnMultiples>false</AbortOnMultiples>
  </global>
  <chocoStatus>
    <WaitOnMultiple>true</WaitOnMultiple>
	<PauseSeconds>30</PauseSeconds>
	<AbortSeconds></AbortSeconds>
  </chocoStatus>
  <PendingRebootStatus>
    <action></action>
  </PendingRebootStatus>
  <WindowsInstallerStatus>
    <WaitOnMultiple>true</WaitOnMultiple>
	<PauseSeconds>30</PauseSeconds>
    <AbortSeconds></AbortSeconds>	
  </WindowsInstallerStatus>
</Settings>

<!--
Chocolatey-Preinstaller-Checks.xml preferences notes:

AbortOnMultiples - true  = abort install/upgrades due to multiple instances of choco or Windows Installer
                 - false = continue as normal
WaitOnMultiple   - true = pause for number of PauseSeconds in hope the other instance(s) will finish
                 - false = warn only and continue as normal
PauseSeconds     - number of seconds to wait and then retry
AbortSeconds     - not implemented
(PendingRebootStatus) action - not implemented, possibly continue, abort, retry (later), reboot

-->

[mwallner]

0.0.2 - Added configurable options to only warn or pause and retry when multiple instances are found.

I think this should be enabled by default / "invert" the switch - so the default behavior doesn't change (make it an explicit choice).

Additionaly (sorry - I haven't checked your code yet) - does your code check whether the "already executing choco.exe processes" are child processes of the one currently running/executing those checks?

[bcurran3]

@mwallner

I think this should be enabled by default / "invert" the switch - so the default behavior doesn't change (make it an explicit choice).

I would have to disagree. Doing so would neuter the extension and it might as well not be installed. It's major purpose is to prevent problems, not just warn. Warnings do nothing for me unless I'm watching upgrades (generally I watch INSTALLS), I don't sit around and watch upgrades - they're automated: https://chocolatey.org/packages/choco-upgrade-all-at. It makes more sense to me to have it take action (i.e. wait for problems to go away and then continue) then to do nothing. Anyone who would want the warning only feature could just configure it that way.

does your code check whether the "already executing choco.exe processes" are child processes of the one currently running/executing those checks?

Yes. In Get-chocoStatus, which calls Get-chocoCounts

# exclude current instance from status report
$chocoInstances = $chocoInstances-1

# chocolatey-preinstaller-checks.extension by Bill Curran AKA BCURRAN3 - 2018 public domain
# get instances of choco.exe and all shims 
# \ProgramData\chocolatey\bin\choco.exe (in path) IS a shim to \ProgramData\chocolatey\choco.exe actual (not in path)
# i.e. running choco.exe will always count as 2 instances
# all shims run choco.exe, so choco.exe count is +1 from the shim execution

function Get-chocoCounts{
$chocoInstances      = @(Get-Process -ea silentlycontinue choco).count #includes choco.exe shim
$chocolateyInstances = @(Get-Process -ea silentlycontinue chocolatey).count
$cinstInstances      = @(Get-Process -ea silentlycontinue cinst).count
$clistInstances      = @(Get-Process -ea silentlycontinue clist).count
$cpackInstances      = @(Get-Process -ea silentlycontinue cpack).count
$cpushInstances      = @(Get-Process -ea silentlycontinue cpush).count
$cuninstInstances    = @(Get-Process -ea silentlycontinue cuninst).count
$cupInstances        = @(Get-Process -ea silentlycontinue cup).count
$cverInstances       = @(Get-Process -ea silentlycontinue cver).count
# add shims to false choco.exe count
$chocoInstances = ($chocoInstances +$chocolateyInstances +$cinstInstances +$clistInstances +$cpackInstances +$cpushInstances +$cuninstInstances +$cupInstances +$cverInstances)
# divide in half to remove shims from count and get real choco.exe instance count
$chocoInstances = $chocoInstances/2
return $chocoInstances
}

[mwallner]

@bcurran3 - I don't think we're getting anywhere concerning the warning/hard-stop topic ... but I get your point.

From my point of view Get-ChocoCounts doesn't solve the problem with choco-child-processes.

Consider following scenario:
You've got a package that's somehow more "dynamic" - i.e. it pulls some code from a server/UNC-share and than executes that code. (not talking about good package design here - I know for a fact that such scenarios exist.) - in this "dynamic code" there may be calls to choco.exe - installing other packages / uninstalling legacy packages etc. (in fact, that's the reason I needed to include a global mutex for reading/writing chocolatey.config last year)
With your extension, at it's current state, it'd be really easy to be stuck in an "endless loop" (until it times out) waiting for the "parent choco.exe" to exit before the "child choco.exe" starts working.

My suggestion: recursively check if the executing choco.exe/cinst.exe are a parent of the current executing one.

i.e. check

Get-CimInstance -Class Win32_Process -Filter "Name = 'choco.exe'" | % { $_.ParentProcessId }

Futhermore, I wouldn't suggest a hard name check on the choco shims & executables .. it'd be way more stable if you check all open handles in the chocolatey /bin directory. (I tend to use sysinternals "listdlls" for such tasks.)

$listDlls = Join-Path $PSScriptRoot "sysinternals\Listdlls.exe"
$dllsInUse = $($(& $listDlls -accepteula) -match [System.Text.RegularExpressions.Regex]::Escape($Path) + ".*\.exe")
if ($dllsInUse.Count) {
  $dllsInUse | ForEach-Object {
    $dllInUse = $_
    $posInString = $dllInUse.IndexOf($Path)
    if ($posInString -eq -1) {
      return;
    }
    $exePath = $dllInUse.Substring($posInString, $dllInUse.Length - $posInString)
    Get-Process | Where-Object { $_.MainModule.FileName -eq "$exePath" } | ForEach-Object { 
      # $_is a process originating from $exePath
    }
  }
}

Hope that helps / lets you understand the "reach" of such a change in the default behavior of choco.exe.

[bcurran3]

@mwallner

I'm not understanding you on the suggested scenario. We can move this to Gitter/e-mail/Issue on my GitHub/wherever if we're too much of a "distraction" here.

So that I can try to understand the "dynamic" scenario:
Are you saying these scripts would have multiple Install-ChocolateyInstallPackage and/or Uninstall-ChocolateyPackage calls? I ask because that's where this extension intercepts and checks for multiple instances of choco.exe (and the other stuff). Infinite loop is not possible using v0.0.2 configured with WaitOnMultiple=false, it would warn only and continue as normal. In this scenario the extension is passive and informational only.

I'm all for making it better. If you want to add to it, please do! PR's welcome.

Hope that helps / lets you understand the "reach" of such a change in the default behavior of choco.exe.

There is no changing of any behavior of choco.exe. There's changing of install/uninstall scripts "behavior."

Do me a favor though: Install, test, and look at the methodology and flow. (If you have, thanks! If not yet, please do.)

Keep in mind also that this extension is 100% opt in. There is no scenario where it would be used as a dependency.

[mikeboutch]

I wrote a new function that support Self-Serve and probably the central management (didn't test it yet ...)

function isUniqueChocoInstance( ) {
  $currentParentProcess=Get-Process -Id (Get-CimInstance -Class Win32_Process -Filter "ProcessID=$($PID)").ParentProcessId
  $chocoCommand='chocolatey', 'choco', 'cinst', 'clist',  'cpack', 'cpush', 'cup', 'cuninst', 'cver', 'chocolatey-management-service' 
  
  $isSelfServe=$False
  if ($currentParentProcess.ProcessName -eq 'chocolatey-agent'){
      $isSelfServe=$True
      Write-verbose "Running in SelfServe"
  }

  Write-verbose "Searching for Choco.exe process"
  $count=0
  Get-Process -ea silentlycontinue -Name choco| ForEach-Object {
    $_ParentName=$(Get-Process -Id (Get-CimInstance -Class Win32_Process -Filter "ProcessID=$($_.Id)").ParentProcessId).ProcessName
    if ($_.Id -eq $PID -or $_.Id -eq $currentParentProcess.Id){
        Write-verbose "$($_.Id) same process or parent of the current"
    } elseif ($_ParentName -eq 'chocolatey-agent'){
        Write-Verbose "$($_.Id) is choco in background mode"    
    } elseif ($chocoCommand.contains($_ParentName)) {
        Write-Verbose "$($_.Id) is choco"
        $count+=1 
    } else {
      Write-Verbose "$($_.Id) is probably a shim of choco"
    }
  }
  Write-Verbose "count is $count"
  if ($isSelfServe){
      $count-=1
  }
  if ($count -gt 0) {
      return $False
  }
  return $True
}


if (isUniqueChocoInstance) {
  Write-Warning "Is Unique"

} else {
  Write-Warning "Not unique"
}