Fix race causing futures to be forgotten too early

[gavtroy]

We were using multiple .done() checks as a condition for processing a group of futures and then deleting them from the dict. The problem is that futures can finish right after a .wait(), meaning they would be .done() but not yet in done_futures. Instead they'd surprise us by being in done_futures after the next .wait(), leading to the error:

File "init.py", line 607, in process_recursive other_dirfutures, = futures[done_future] KeyError: <Future at 0x7fbfe041adf0 state=finished returned tuple>

After this, scanning continued but tagging did not.

[coveralls]

Coverage decreased (-0.1%) to 74.86% when pulling 4a9c7348c9d8d04811cb58f29e03a0b154c96432 on gavtroy:dev_missing_future into 9276e30ca4b80cfce720b2348682d8a433a67af3 on desbma:master.

[desbma]

Nice catch!

Thanks for taking the time to fix this, and writing a clear explanation. :+1:

[gavtroy]

No problem. I should also have pointed out that this is extremely unlikely to happen in album mode, which I assume almost everybody uses!

[topas-rec]

Have the same error occasionally. (I use album mode and without the "ignore tagged" option (-s) I'm unable to get about 1500 files tagged) @desbma can we get a pypi update? I don't like installing from source, since it seems pain to uninstall it. @gavtroy is it safe to run r128gain multiple times with the ignore tagged option (- s) enabled when this issue occurs? ( It seems you dived in deeply). Are there files left in an inconsistent tagged state when this issue occurs?

[gavtroy]

Now that you say that I can imagine some scenarios where it could happen in album mode too, but I'm still surprised it happens easily. I wonder if this is the same issue that you see.

It shouldn't result in any incorrect metadata being written so resuming with -s should be fine. You can also avoid installing if you cd to the root of the repo and run "python -m r128gain -r -a ..."; just be aware it will pick up the system install if you're in the wrong directory.

[topas-rec]

I wonder if this is the same issue that you see.

Looks like I have to test this PR/commit.

shouldn't result in any incorrect metadata being written so resuming with -s should be fine.

Perfect thanks.

You can also avoid installing if you cd to the root of the repo and run "python -m r128gain -r -a ..."; just be aware it will pick up the system install if you're in the wrong directory.

Thanks, this makes testing easier.

[topas-rec]

I did

r128gain -a -r -c 4 .

about seven times with version 1.0.2 without a single successful complete run. (Incomplete runs ended with the mentioned message above)

I tried the current master branch (current commit https://github.com/desbma/r128gain/commit/aafe2526e836b6ded6e50eb439b7dfe8441c04ed) with the same files and r128gain successfully measured and tagged all files both times without failure.

I can do test further if you want. Thanks @desbma and @gavtroy. 👏

[desbma]

Thanks @topas-rec fort checking the fix in the master branch.

I'll make a new release soon.