chocolatey / choco

Chocolatey - the package manager for Windows
https://chocolatey.org
Other
10.35k stars 903 forks source link

chocolatey.config gets corrupted when multiple processes access simultaneously #1047

Closed mwallner closed 7 years ago

mwallner commented 8 years ago

First of all, I do know this problem seems to be related to ChocolateyGUI (read on) - but I think the error is in chocolatey.dll - so I decided to post this Issue here.

chocolatey.config xml gets corrupted

When starting chocolateyGUI while another process uses choco (like choco.exe from command-line) - chocolatey.config will be corrupted and can't be read any longer. - invalid xml, the XML serializer just seems to stop writing the file.

example of .config before the issue occurs

<?xml version="1.0" encoding="utf-8"?>
<chocolatey xmlns:xsd="http://www.w3.org/2001/XMLSchema" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance">
  <containsLegacyPackageInstalls>false</containsLegacyPackageInstalls>
  <commandExecutionTimeoutSeconds>0</commandExecutionTimeoutSeconds>
  <config>
    <add key="cacheLocation" value="" description="Cache location if not TEMP folder." />
    <add key="containsLegacyPackageInstalls" value="true" description="Install has packages installed prior to 0.9.9 series." />
    <add key="commandExecutionTimeoutSeconds" value="2700" description="Default timeout for command execution." />
    <add key="proxy" value="" description="Explicit proxy location." />
    <add key="proxyUser" value="" description="Optional proxy user." />
    <add key="proxyPassword" value="" description="Optional proxy password. Encrypted." />
    <add key="webRequestTimeoutSeconds" value="30" description="Default timeout for web requests. Available in 0.9.10+." />
  </config>
  <sources>
    <source id="private-server" value="https://................../" disabled="false" priority="1" />
  </sources>
  <features>
    <feature name="checksumFiles" enabled="true" setExplicitly="false" description="Checksum files when pulled in from internet (based on package)." />
    <feature name="autoUninstaller" enabled="true" setExplicitly="false" description="Uninstall from programs and features without requiring an explicit uninstall script." />
    <feature name="allowGlobalConfirmation" enabled="false" setExplicitly="false" description="Prompt for confirmation in scripts or bypass." />
    <feature name="failOnAutoUninstaller" enabled="false" setExplicitly="false" description="Fail if automatic uninstaller fails." />
    <feature name="failOnStandardError" enabled="false" setExplicitly="false" description="Fail if install provider writes to stderr. Available in 0.9.10+." />
    <feature name="powershellHost" enabled="true" setExplicitly="false" description="Use Chocolatey's built-in PowerShell host. Available in 0.9.10+." />
    <feature name="logEnvironmentValues" enabled="false" setExplicitly="false" description="Log Environment Values - will log values of environment before and after install (could disclose sensitive data). Available in 0.9.10+." />
    <feature name="virusCheck" enabled="false" setExplicitly="false" description="Virus Check - perform virus checking on downloaded files. Available in 0.9.10+. Licensed versions only." />
    <feature name="failOnInvalidOrMissingLicense" enabled="false" setExplicitly="false" description="Fail On Invalid Or Missing License - allows knowing when a license is expired or not applied to a machine. Available in 0.9.10+." />
    <feature name="ignoreInvalidOptionsSwitches" enabled="true" setExplicitly="false" description="Ignore Invalid Options/Switches - If a switch or option is passed that is not recognized, should choco fail? Available in 0.9.10+." />
    <feature name="usePackageExitCodes" enabled="true" setExplicitly="false" description="Use Package Exit Codes - Package scripts can provide exit codes. With this on, package exit codes will be what choco uses for exit when non-zero (this value can come from a dependency package). Chocolatey defines valid exit codes as 0, 1605, 1614, 1641, 3010. With this feature off, choco will exit with a 0 or a 1 (matching previous behavior). Available in 0.9.10+." />
    <feature name="useFipsCompliantChecksums" enabled="false" setExplicitly="false" description="Use FIPS Compliant Checksums - Ensure checksumming done by choco uses FIPS compliant algorithms. Not recommended unless required by FIPS Mode. Enabling on an existing installation could have unintended consequences related to upgrades/uninstalls. Available in 0.9.10+." />
    <feature name="allowEmptyChecksums" enabled="true" setExplicitly="true" description="Allow packages to have empty/missing checksums for downloaded resources from non-secure locations (HTTP, FTP). Enabling is not recommended if using sources that download resources from the internet. Available in 0.10.0+." />
    <feature name="allowEmptyChecksumsSecure" enabled="true" setExplicitly="false" description="Allow packages to have empty/missing checksums for downloaded resources from secure locations (HTTPS). Available in 0.10.0+." />
    <feature name="scriptsCheckLastExitCode" enabled="false" setExplicitly="false" description="Scripts Check $LastExitCode (external commands) - Leave this off unless you absolutely need it while you fix your package scripts  to use `throw 'error message'` or `Set-PowerShellExitCode #` instead of `exit #`. This behavior started in 0.9.10 and produced hard to find bugs. If the last external process exits successfully but with an exit code of not zero, this could cause hard to detect package failures. Available in 0.10.3+. Will be removed in 0.11.0." />
  </features>
  <apiKeys />
</chocolatey>

example of .config afterwards

<?xml version="1.0" encoding="utf-8"?>
<chocolatey xmlns:xsd="http://www.w3.org/2001/XMLSchema" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance">
  <containsLegacyPackageInstalls>false</containsLegacyPackageInstalls>
  <commandExecutionTimeoutSeconds>0</commandExecutionTimeoutSeconds>
  <config>
    <add key="cacheLocation" value="" description="Cache location if not TEMP folder." />
    <add key="containsLegacyPackageInstalls" value="true" description="Install has packages installed prior to 0.9.9 series." />
    <add key="commandExecutionTimeoutSeconds" value="2700" description="Default timeout for command execution." />
    <add key="proxy" value="" description="Explicit proxy location." />
    <add key="proxyUser" value="" description="Optional proxy user." />
    <add key="proxyPassword" value="" description="Optional proxy password. Encrypted." />
    <add key="webRequestTimeoutSeconds" value="30" description="Default timeout for web requests. Available in 0.9.10+." />
  </config>
  <sources>
    <source id="private-server" value="https://................../" disabled="false" priority="1" />
  </sources>
  <features>
    <feature name="checksumFiles" enabled="true" setExplicitly="false" description="Checksum files when pulled in from internet (based on package)." />
    <feature name="autoUninstaller" enabled="true" setExplicitly="false" description="Uninstall from programs and features without requiring an explicit uninstall script." />
    <feature name="allowGlobalConfirmation" enabled="false" setExplicitly="false" description="Prompt for confirmation in scripts or bypass." />
    <feature name="failOnAutoUninstaller" enabled="false" setExplicitly="false" description="Fail if automatic uninstaller fails." />
    <feature name="failOnStandardError" enabled="false" setExplicitly="false" description="Fail if install provider writes to stderr." />
    <feature name="powershellHost" enabled="true" setExplicitly="false" description="Use Chocolatey's built-in PowerShell host." />
    <feature name="logEnvironmentValues" enabled="false" setExplicitly="false" description="Log Environment Values - will log values of environment before and after install (could disclose sensitive data)." />
    <feature name="virusCheck" enabled="false" setExplicitly="false" description="Virus Check [licensed versions only] - perform virus checking on downloaded files." />
    <feature name="failOnInvalidOrMissingLicense" enabled="false" setExplicitly="false" description="Fail On Invalid Or Missing License - allows knowing when a license is expired or not applied to a machine." />
    <feature name="ignoreInvalidOptionsSwitches" enabled="true" setExplicitly="false" description="Ignore Invalid Options/Switches - If a switch or option is passed that is not recognized, should choco fail?" />
    <feature name="usePackageExitCodes" enabled="true" setExplicitly="false" description="Use Package Exit Codes - Package scripts can provide exit codes. With this on, package exit codes will be what choco uses for exit when non-zero (this value can come from a dependency package). Chocolatey defines valid exit codes as 0, 1605, 1614, 1641, 3010. With this feature off, choco will exit with a 0 or a 1 (matching previous behavior). Available in 0.9.10+." />
    <feature name="useFipsCompliantChecksums" enabled="false" setExplicitly="false" description="Use FIPS Compliant Checksums - Ensure checksumming done by choco uses FIPS compliant algorithms. Not recommended unless required by FIPS Mode. Enabling on an existing installation could have unintended consequences related to upgrades/uninstalls. Available in 0.9.10+." />
    <feature name="allowEmptyChecksums" enabled="true" setExplicitly="true" description="Allow packages to have empty/missing checksums for downloaded resources from non-secure locations (HTTP, FTP). Enabling is not recommended if using sources that download resources from the internet. Available in 0.10.0+." />
    <feature name="allowEmptyChecksumsSecure" enabled="true" setExplicitly="false" description="Allow packages to have empty/missing checksums for downloaded resources from secure locations (HTTPS). Available in 0.10.0+." />
    <feature name="scriptsCheckLastExitCode" enabled="false" setExplicitly="false" description="Scripts Check $LastExitCode (external commands) - Leave this off unless you absolutely need it while you fix your package scripts  to use `throw 

once this occurs, chocolatey can't be used anymore:

the output choco list -lo -verbose -debug :

Error deserializing response of type chocolatey.infrastructure.app.configuration.ConfigFileSettings:
 There is an error in XML document (32, 242).
Chocolatey had an error occur:
System.InvalidOperationException: There is an error in XML document (32, 242). ---> System.Xml.XmlException: There is an unclosed literal string. Line 32, position 242.
   at System.Xml.XmlTextReaderImpl.Throw(Exception e)
   at System.Xml.XmlTextReaderImpl.ParseAttributeValueSlow(Int32 curPos, Char quoteChar, NodeData attr)
   at System.Xml.XmlTextReaderImpl.ParseAttributes()
   at System.Xml.XmlTextReaderImpl.ParseElement()
   at System.Xml.XmlTextReaderImpl.ParseElementContent()
   at System.Xml.XmlReader.MoveToContent()
   at Microsoft.Xml.Serialization.GeneratedAssembly.XmlSerializationReaderConfigFileSettings.Read6_ConfigFileSettings(Boolean isNullable, Boolean checkType)
   at Microsoft.Xml.Serialization.GeneratedAssembly.XmlSerializationReaderConfigFileSettings.Read7_chocolatey()
   --- End of inner exception stack trace ---
   at System.Xml.Serialization.XmlSerializer.Deserialize(XmlReader xmlReader, String encodingStyle, XmlDeserializationEvents events)
   at chocolatey.infrastructure.services.XmlService.<>c__DisplayClass1`1.<deserialize>b__0()
   at chocolatey.infrastructure.tolerance.FaultTolerance.try_catch_with_logging_exception[T](Func`1 function, String errorMessage, Boolean throwError, Boolean logWarningInsteadOfError, Boolean logDebugInsteadOfError, Boolean isSilent)
   at chocolatey.infrastructure.services.XmlService.deserialize[XmlType](String xmlFilePath)
   at chocolatey.infrastructure.app.builders.ConfigurationBuilder.set_up_configuration(IList`1 args, ChocolateyConfiguration config, Container container, ChocolateyLicense license, Action`1 notifyWarnLoggingAction)
   at chocolatey.console.Program.Main(String[] args)

chocolatey.config should only be read, not written every time ChocolateySources are read via chocolatey.dll

Why does chocolatey.config need to be written every time ChocolateyGUI is opened?

steps to reproduce

screenshots

choco_bug

Having a look at the soures of chocolateyGUI reveals that this happens during initialization of chocolatey.dll choco_bug_get_sources

mwallner commented 8 years ago

The error seems to be in chocolatey/infrastructure/services/XmlService.cs I can reproduce the _fileSystem.copy_file to be ?unsuccessful? (only a certain amount of data is replaced within chocolatey.config) - afterwards xmlUpdateFilePath gets deleted. Maybe a timing issue? (xmlUpdateFilePath should only be deleted after _fileSystem.copy_file has finished..)

image

ferventcoder commented 8 years ago

@mwallner note that this writes a file called chocolatey.config.update, then it compares this file versus the original to see if it has changed (line 86 in your image). Only then does it copy over the existing.

I think possibly we need to read the file back in to see if it is valid prior to overwriting. What do you think?

mwallner commented 8 years ago

@ferventcoder as far as I've tracked the bug it seems to be a timing issue .. I think the file isn't really written synchronously with _fileSystem.copy_file - and _fileSytem.delete_file stops the copy-operation somewhere during it's runtime..

I'd suggest to only delete the .update file if the content of the .config file and the .update file match

ferventcoder commented 7 years ago

Fixed for 0.10.4

mwallner commented 7 years ago

Please reopen, still an Issue in 0.10.5

here's a sample of chocolatey.config after two processes accessed it simultaneously:

<?xml version="1.0" encoding="utf-8"?>
<chocolatey xmlns:xsd="http://www.w3.org/2001/XMLSchema" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance">
  <containsLegacyPackageInstalls>false</containsLegacyPackageInstalls>
  <commandExecutionTimeoutSeconds>0</commandExecutionTimeoutSeconds>
  <config>
    <add key="cacheLocation" value="" description="Cache location if not TEMP folder." />
    <add key="containsLegacyPackageInstalls" value="true" description="Install has packages installed prior to 0.9.9 series." />
    <add key="commandExecutionTimeoutSeconds" value="2700" description="Default timeout for command execution." />
    <add key="

1 good thing though: now there's a .config.backup and config.update file as well - still containing the correct configuration.

tek0011 commented 7 years ago

Isnt it re-referenced here?

https://github.com/chocolatey/choco/issues/1241

oh nope nevermind. this ticket is linked to that, but closed. I do still have this issue in 10.5 as well. We just moved all our hosts back to 10.3 for the time being.

ferventcoder commented 7 years ago

The fixes that went into for this are supposed to look at the file after it saves it and determine if it is good - if not it should roll back old config.

ferventcoder commented 7 years ago

Let's open a new issue and reference this one.