Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cache_dir_lock in main is always None, so the lockfile never gets cleaned up #4030

Closed
guppy0130 opened this issue Feb 16, 2024 · 1 comment · Fixed by #4055
Closed

cache_dir_lock in main is always None, so the lockfile never gets cleaned up #4030

guppy0130 opened this issue Feb 16, 2024 · 1 comment · Fixed by #4055
Labels

Comments

@guppy0130
Copy link
Contributor

Summary

cache_dir_lock is initialized as None here, in the global scope I believe:

cache_dir_lock: None | FileLock = None

In initialize_options, a new, frame-only cache_dir_lock is created here:

cache_dir_lock = FileLock( # pylint: disable=redefined-outer-name
f"{options.cache_dir}/.lock",
)
try:
cache_dir_lock.acquire(timeout=180)

I think pylint was correct to indicate that the global name was redefined. This variable isn't marked as global and so the cache_dir_lock in initialize_options is local to initialize_options only.

The value of cache_dir_lock (the global one) is still None later on, so this conditional is always falsy and the lockfile therefore never gets cleaned up:

if cache_dir_lock:
cache_dir_lock.release()
pathlib.Path(cache_dir_lock.lock_file).unlink(missing_ok=True)

I believe this behavior is introduced by c6bd9a7.

I'd love to PR to resolve this, but I don't know if you prefer the global approach or just returning the lock from initialize_options.

Issue Type
  • Bug Report
OS / ENVIRONMENT
$ ansible-lint --version
ansible-lint 24.2.0 using ansible-core:2.16.3 ansible-compat:4.1.11 ruamel-yaml:0.18.6 ruamel-yaml-clib:0.2.8
  • ansible installation method: pip
  • ansible-lint installation method: pip
STEPS TO REPRODUCE

In one session

$ rm -r ~/.cache/ansible-compat/*
$ watch -n 1 'ls ~/.cache/ansible-compat/**/.lock'  # the quotes are so the shell under watch will expand it, not your current shell

and in another session

$ pushd /tmp
$ mkdir blah
$ vim main.yml  # see below for MCVE playbook, but it can really be any playbook
$ ansible-lint

watch the lockfile get created when ansible-lint is run, and then it shouldn't get cleaned up.

Desired Behavior

The cache_dir_lock that main uses is not None, either by the global keyword or initialize_options returning it to its caller, main; in which case, the lock can be correctly cleaned up after ansible-lint finishes.

Actual Behavior

Ansible-lint doesn't clean up the lockfile, because cache_dir_lock is None in main.

Please give some details of what is happening. Include a minimum complete
verifiable example
with:

  • minimized playbook to reproduce the error
- name: "MCVE"
  gather_facts: false
  hosts: "localhost"
  tasks:
    - name: "Hello world"
      ansible.builtin.debug:
        msg: "hello world"
  • the output of running ansible-lint including the command line used
Passed: 0 failure(s), 0 warning(s) on 1 files. Last profile that met the validation criteria was 'production'.
@guppy0130 guppy0130 added bug new Triage required labels Feb 16, 2024
@ssbarnea ssbarnea removed the new Triage required label Feb 28, 2024
@ssbarnea
Copy link
Member

Seems legit, a pull-requests would be welcomed.

guppy0130 added a commit to guppy0130/ansible-lint that referenced this issue Mar 2, 2024
* in `initialize_options`, we need to update the globally-scoped
  `cache_dir_lock` so that later on, `main` can release the lock and
  cleanup the lockfile.

Fixes ansible#4030
guppy0130 added a commit to guppy0130/ansible-lint that referenced this issue Mar 2, 2024
* in `initialize_options`, we need to update the globally-scoped
  `cache_dir_lock` so that later on, `main` can release the lock and
  cleanup the lockfile.

Fixes ansible#4030
audgirka pushed a commit to guppy0130/ansible-lint that referenced this issue Mar 4, 2024
* in `initialize_options`, we need to update the globally-scoped
  `cache_dir_lock` so that later on, `main` can release the lock and
  cleanup the lockfile.

Fixes ansible#4030
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants