Skip to content

(#2888) Fix race condition with puppetdlockfile#1158

Merged
jeffmccune merged 2 commits intopuppetlabs:2.7.xfrom
jeffmccune:bug/2.7.x/2888_fix_puppetdlock_deadlock
Sep 17, 2012
Merged

(#2888) Fix race condition with puppetdlockfile#1158
jeffmccune merged 2 commits intopuppetlabs:2.7.xfrom
jeffmccune:bug/2.7.x/2888_fix_puppetdlock_deadlock

Conversation

@jeffmccune
Copy link
Contributor

Without this patch applied there is a race condition where by two or
more concurrent puppet agent processes will eventually result in a
deadlock state where all processes consider catalog runs to be
administratively disabled.

This is a problem because the configuration catalog stops being applied
periodically. The system will not recover unless there is manual
intervention by the end user.

The problem is caused by the settings catalog. The settings catalog
will contain a file resource for the puppetdlockfile if the file exists.
When two processes are running, the file will exist when it is created
by the other process. When the file resource is in the settings
catalog, it will be created as a zero length file, or truncated to a
zero length file if it already exists.

Here is the state transition for two puppet agent --test processes, A
and B.

A: (Start of the catalog run, after settings catalog is applied)
lock()
  File.exist? => false
  File.open()
  catalog.run
  ...

B: (Managing settings catalog, before user catalog run)
catalog.add_resource(Puppet::Type::File)
  Puppet::Util::Settings#to_catalog
  ...
  resource synchronize
  Puppet::Type::File#write(:content)

A:
unlock()
  unlink()

B:
#<Puppet::Resource::Catalog[Settings]> apply
  File.open("puppetdlock", "wb") { ... } # Truncates the file!A

A: run
  locked? => true
  mine? => false

B: run
  locked? => true
  mine? => false

A and B are deadlocked.

This patch fixes the problem by marking the puppetdlockfile settings as
a :type => :setting. This change prevents the settings catalog from
containing a file resource for the puppetdlockfile setting.

Paired-with: Josh Cooper josh@puppetlabs.com

Jeff McCune added 2 commits September 17, 2012 14:34
Without this patch the README_DEVELOPER doesn't mention the special
settings catalog which I overlooked while working on puppetlabs#2888.  If the
settings catalog was in mind while I worked on this issue I probably
would have solved it a lot faster than it took to track down.

In particular, wrapping the File.open method was an effective way to
quickly track down the codepath that was improperly writing and
truncating the lockfile.
Without this patch applied there is a race condition where by two or
more concurrent puppet agent processes will eventually result in a
deadlock state where all processes consider catalog runs to be
administratively disabled.

This is a problem because the configuration catalog stops being applied
periodically.  The system will not recover unless there is manual
intervention by the end user.

The problem is caused by the settings catalog.  The settings catalog
will contain a file resource for the puppetdlockfile if the file exists.
When two processes are running, the file will exist when it is created
by the other process.  When the file resource is in the settings
catalog, it will be created as a zero length file, or truncated to a
zero length file if it already exists.

Here is the state transition for two puppet agent --test processes, A
and B.

    A:
    lock()
      File.exist? => false
      File.open()
      catalog.run
      ...

    B:
    Puppet::Util::Settings#to_catalog
      File.exist? => true
      catalog.add_resource(Puppet::Type::File)
      ...
      resource synchronizes
      Puppet::Type::File#write(:content)

    A:
    unlock()
      unlink()

    B:
    settings catalog apply
      File.open("puppetdlock", "wb") { ... } # Truncates the file!

    A:
    locked? => true
    mine? => false

    B:
    locked? => true
    mine? => false

    A and B are deadlocked.

This patch fixes the problem by marking the puppetdlockfile settings as
a `:type => :setting`.  This change prevents the settings catalog from
containing a file resource for the puppetdlockfile setting.

Paired-with: Josh Cooper <josh@puppetlabs.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant