phive icon indicating copy to clipboard operation
phive copied to clipboard

Extra content at the end of the document in `.phive/registry.xml`

Open boesing opened this issue 2 years ago • 3 comments

Hey there,

we are using phive on our CI pipeline. We do have plenty of projects with plenty of build steps and many of these build steps do use phive to install dependencies such as psalm, phpunit, composer, and so on.

I recently had a phive error due to the fact, that the registry.xml looked like this:

<?xml version="1.0" encoding="UTF-8"?>
<phars xmlns="https://phar.io/phive/installdb">
  <phar name="composer" version="2.1.8" location="/home/integration/.phive/phars/composer-2.1.8.phar">
    <hash type="sha1">d3115f774104d42b598182ddf3c2e36d5fe8001d</hash>
    <signature fingerprint="161DFBE342889F01DDAC4E61CBB3D576F2A0946F"/>
    <usage destination="/home/integration/agent-6/xml-data/build-dir/MH-MX-JOB1/source/tools/composer"/>
  </phar>
</phars>
>

I can only guess that this happened due to the fact that multiple processes written the same file at the same time.

I have a few questions regarding this, just to outline if we just use phive wrong:

  • Is there a chance that phive does not lock the file before writing it?
  • Is there any suggestion on how to handle this? Because even tho, that file would be locked, this could lead to race conditions when multiple runs actually write the same file with possibly different data.

So we do not have a per-user environment per-project and thus, using the users $HOME might not be the best solution for us. Is there any way of avoiding that registry file at all and just act as it is always missing?

Thanks in advance and with regards from Cologne Max



Phive 0.15.0 - Copyright (C) 2015-2022 by Arne Blankerts, Sebastian Heuer and Contributors
[ERROR]   An error occurred while processing your request:

          DOMDocument::load(): Extra content at the end of the document in /home/integration/.phive/registry.xml, line: 616
          
          #0 src/shared/XmlFile.php(104)
          #1 unknown file(0): PharIo\Phive\Cli\Runner->errorHandler()
          #2 src/shared/XmlFile.php(104): DOMDocument->load()
          #3 src/shared/XmlFile.php(84): PharIo\Phive\XmlFile->initDom()
          #4 src/shared/XmlFile.php(64): PharIo\Phive\XmlFile->getDom()
          #5 src/shared/PharRegistry.php(75): PharIo\Phive\XmlFile->query()
          #6 src/shared/repository/LocalRepository.php(29): PharIo\Phive\PharRegistry->getPhars()
          #7 src/services/resolver/LocalAliasResolver.php(31): PharIo\Phive\LocalRepository->getReleasesByRequestedPhar()
          #8 src/services/resolver/RequestedPharResolverService.php(37): PharIo\Phive\LocalAliasResolver->resolve()
          #9 src/commands/install/InstallCommand.php(62): PharIo\Phive\RequestedPharResolverService->resolve()
          #10 src/commands/install/InstallCommand.php(51): PharIo\Phive\InstallCommand->resolveToRelease()
          #11 src/commands/install/InstallCommand.php(46): PharIo\Phive\InstallCommand->installRequestedPhar()
          #12 src/shared/cli/Runner.php(241): PharIo\Phive\InstallCommand->execute()
          #13 src/shared/cli/Runner.php(95): PharIo\Phive\Cli\Runner->execute()
          #14 (350): PharIo\Phive\Cli\Runner->run()
          #15 {main}

          Environment: PHP 8.1.0 (on Linux 4.19.0-14-amd64)
          Phive Version: 0.15.0

          This should not have happened and is most likely a bug.
          Please report it at https://github.com/phar-io/phive/issues, make sure you include
          the full output of this error message. Thank you!

boesing avatar Apr 14 '22 09:04 boesing

Sorry for taking long to investigate. There are multiple angles to this:

  • Phive indeed doesn't do anything in particular to protect itself from race conditions when operating on its files. I admit, so far this didn't seem to be a major concern. For project local files, I still don't see any issues per se as I cannot see a reason for having concurrent jobs operate on these. I can see though that having multiple projects sharing a registry and having processes running in parallel with such a shared registry will be a problem.

  • Simply locking the file(s) though won't be enough. We read the registry and operate on it during a phive run. A write lock won't be enough as we would have the last process to finish win, overwriting changes other processes made. We would have to keep the lock open during the full phive run, basically blocking all concurrency (like dnf or yum). Or in other words: We'd need to ensure we only have one process at a time running.

  • If we want to support concurrency like htis, we probably need to change the persistence layer of the registry. E.g. migrate to sqlite where we can simply update records without rewriting the full registry file. That's not a trivial change though ;)

  • That being said, I'm confused as to how we manage to actually break the xml file like shown: We use DOMDocument::save to persist the file and it surprises me that if a race condition like this exists the "only" thing broken is the superfluous > at the end. I don't see any other logical reason though either...

  • For now, the best approach probably is to avoid having a shared registry. Phive supports specifying an alternative home directory, either by passing --home to it before the command (e.g. phive --home /some/where install ...) or by setting the environment variable PHIVE_HOME.

theseer avatar Apr 19 '22 20:04 theseer

No worries. Thanks for having a look at it! Since we have to assume that the registry is not available during CI runs anyway, I can try to create a temporary directory for the home. Another way could be to pass something like --disable-persistence which actually forces phive to use some kind of VoidPersistence and therefore never has an existing registry.

I will see if I can make that happen with the home directory and if that wont work, would that option be a solution I could contribute?

Let me know what you think. 👌🏻

boesing avatar Apr 19 '22 20:04 boesing

Hm, happened again today - multiple times. We do use phive in multiple projects which are all build on the same machines in different directories but all sharing the same home directory where phive inserts its data.

So depending on the directories the builds are executed, the registry gets a new entry. The directory names are somehow related to the build number (bamboo) and thus are probably changing.

I ended up having a large file containing multiple projects and again, at the end of the file, there was the exact same character again:

<?xml version="1.0" encoding="UTF-8"?>
<phars xmlns="https://phar.io/phive/installdb">
<!-- entries -->
</phars>
>

This is really odd and I have no idea why this is the case. :-/


By using an additional --home, this would disable the shared phars and thus leads to phar downloads for every build if a temporary home is being used only for the current build. So I wonder if there would be an additional way to just disable the registry while keeping all the other stuff at the same location.

boesing avatar Jun 02 '22 09:06 boesing

Just as a little update:

We do actually pass PHAR_HOME as env variable for the specific build directory. That definitely fixed the problem so there are definitely race conditions which lead to the invalid XML file.

boesing avatar Sep 28 '22 16:09 boesing

Thanks for the heads up! I really need to find some time to work on phive...

theseer avatar Sep 28 '22 20:09 theseer

We apparently have multiple logical issues here when running multiple instances in parallel.

As mentioned in PR #379, the best work around until i can fix the underlying issue is to use --temporary to not update the registries. Unfortunately, --temporary currently does update the usage in the PHAR_HOME registry file - which doesn't make sense logically when only "temporary" install things and could qualify as a bug on its own.

We could argue whether or not having it create an entry for the phar in general. If not, we'll redownload the same phar every time - which is useless. But for --temporary, it would make sense not to store things. I still lean towards having it recorded. That would leave the race condition for the PHAR_HOME registry file when installing a new phar for the first time.

So we probably need a way to lock that file for the phive run.

theseer avatar Oct 14 '22 09:10 theseer

Just as a heads-up here. We end-up committing the PHAR-Files to the repositories so that our build pipeline can continue work. Anyway, thanks for taking time to dive deeper into this. Sorry for the inconvenience and enjoy the holidays 👍🏼

boesing avatar Dec 16 '22 19:12 boesing