beetbox / beets

music library manager and MusicBrainz tagger
http://beets.io/
MIT License
12.87k stars 1.82k forks source link

chroma: Abort fingerprinting immediately on SIGINT #3139

Open ssssam opened 5 years ago

ssssam commented 5 years ago

Problem

Steps to reproduce:

  1. Create a directory with lots of MP3 files (at least 10)
  2. Run beet import on the directory.
  3. Press CTRL+C.
  4. Watch for a while as Beets fingerprints every file in the directory before exiting.

The same issue occurs when the 'aBort' option is selected from the autotag prompt.

I reproduced the issue using the following config file:

directory: /tmp/beets-testcase
library: /tmp/beets-testcase/db

plugins: chroma

import:
    timid: yes

Cause

The issue is that the 'chroma' plugin fingerprints every file in the directory during a single fingerprint_file() task. This doesn't get aborted when the pipeline receives an exception, so the Pipeline.run_parallel() method blocks until all the files have been fingerprinted.

As soon as the task finishes all of this data is thrown away due to the exception, so we should really stop processing files as soon as possible here and allow Beets to exit quickly.

sampsyo commented 5 years ago

Sounds good! This would be good behavior to have.

ssssam commented 5 years ago

In fact, even without the 'chroma' plugin it seems like the lookup_candidates() stage could block for a while.

One way to fix this would be to abort Beets without allowing the threads to complete. This could introduce issues like data corruption in tasks which write to files, so it's probably a bad idea.

We could add periodical checks into long running tasks so that they exit if a certain flag is set. What about adding a task.aborted flag, and inserting blocks such as this in places:

if task.aborted:
    return