lockutils - remove lock dir creation and cleanup

[Mark McLoughlin]

See https://review.openstack.org/14139

This:

                    if not local_lock_path:
                        cleanup_dir = True
                        local_lock_path = tempfile.mkdtemp()

                    if not os.path.exists(local_lock_path):
                        cleanup_dir = True
                        ensure_tree(local_lock_path)
                    ...
                    finally:
                        # NOTE(vish): This removes the tempdir if we needed
                        # to create one. This is used to cleanup
                        # the locks left behind by unit tests.
                        if cleanup_dir:
                            shutil.rmtree(local_lock_path)

Why are we deleting the lock dir here? Does that even work? i.e. what if someone concurrently tries to take the lock, re-creates the dir and lock a new file?

[Mark McLoughlin (markmc)]

Ok, this reared its head in the form of bug #1158179

I think we need to move the lock_path directory creation and removal code out of this function altogether and require the callers to do it

Using a tempdir just never makes sense

For a well known path, if this code assumes the responsibility of creating the dir, it should also remove the dir ... but the bug shows that it's not safe to do so

[Zhongyue Luo (zyluo)]

Maybe we need to edit the synchronized_with_prefix to require a lock_path also?

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/36350
Committed: http://github.com/openstack/oslo-incubator/commit/90b6a65545dd2d41d674dd22f00bcd6dea695239
Submitter: Jenkins
Branch: master

commit 90b6a65545dd2d41d674dd22f00bcd6dea695239
Author: Ben Nemec <email address hidden>
Date: Tue Jul 9 17:03:53 2013 -0500

    Fix locking bug

    When lock_path is not set, lockutils creates a new temp dir for
    each new call to synchronized. This results in no actual lock
    enforcement. Require setting of lock_path by throwing an
    exception if it is not found.

    Fixes bug 1065531
    Fixes bug 1162047

    Change-Id: I178684a1d8649b5bcfcb768be0a68c8efa3f00e4

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/38685
Committed: http://github.com/openstack/nova/commit/d7aee2352c023401b70e4cd930cde5c08a9374d5
Submitter: Jenkins
Branch: master

commit d7aee2352c023401b70e4cd930cde5c08a9374d5
Author: Ben Nemec <email address hidden>
Date: Thu Jul 25 10:56:55 2013 -0500

    Sync lockutils from Oslo

    90b6a65 Fix locking bug
    27d4b41 Move synchronized body to a first-class function
    15c17fb Make lock_file_prefix optional
    1a2df89 Enable H302 hacking check
    b41862d Use param keyword for docstrings

    Fixes bug 1065531
    And bug 1162047

    Change-Id: Ide79292fae6f779ecd4ac166d68c8f10ca728409

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/40662

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/40663

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/40662
Committed: http://github.com/openstack/cinder/commit/7d9e0dead3f668f5ad3e663cd342f3c2c3af9a5d
Submitter: Jenkins
Branch: master

commit 7d9e0dead3f668f5ad3e663cd342f3c2c3af9a5d
Author: Ben Nemec <email address hidden>
Date: Wed Aug 7 10:54:34 2013 -0500

    Set lock_path in tests

    This is required to sync a locking fix from Oslo.

    Change-Id: I1f71d7137eab2509a24c5e1397da653142561f10
    Related-Bug: 1065531

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/47557
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=eece55ceb687c425de1066851c9601221f1ef2b7
Submitter: Jenkins
Branch: master

commit eece55ceb687c425de1066851c9601221f1ef2b7
Author: Ann Kamyshnikova <email address hidden>
Date: Fri Sep 20 15:48:37 2013 +0400

    Update lockutils and fixture in openstack.common

    lockutils: included commits:
      8b2b0b7 Use hacking import_exceptions for gettextutils._
      6d0a6c3 Correct invalid docstrings
      12bcdb7 Remove vim header
      79e6bc6 fix lockutils.lock() to make it thread-safe
      ace5120 Add main() to lockutils that creates temp dir for locks
      537d8e2 Allow lockutils to get lock_path conf from envvar
      371fa42 Move LockFixture into a fixtures module
      d498c42 Fix to properly log when we release a semaphore
      29d387c Add LockFixture to lockutils
      3e3ac0c Modify lockutils.py due to dispose of eventlet
      90b6a65 Fix locking bug
      27d4b41 Move synchronized body to a first-class function
      15c17fb Make lock_file_prefix optional
      1a2df89 Enable H302 hacking check

    fixture: created, included commits:
      45658e2 Fix violations of H302:import only modules
      12bcdb7 Remove vim header
      3970d46 Fix typos in oslo
      371fa42 Move LockFixture into a fixtures module
      f4a4855 Consolidate the use of stubs
      6111131 Make openstack.common.fixture.config Py3 compliant
      3906979 Add a fixture for dealing with config
      d332cca Add a fixture for dealing with mock patching.
      1bc3ecf Start adding reusable test fixtures.

    Also tox.ini was corrected to let lockutils work in tests.

    This change is needed for work on bp: db-sync-models-with-migrations

    Closes-Bug: #1065531

    Change-Id: I139f30b4767ff2c9d1f01ee728823859c09b3859

[Ben Nemec (bnemec)]

I believe everyone is on oslo.concurrency now, so this should no longer be an issue anywhere.