feat: handle CancelledError - cancel if no other waiters

[BobTheBuidler]

What do these changes do?

Currently if a decorated function is awaited, and then that task is cancelled, async_lru will never cancel the task, potentially leading to memory leaks and other fun things.

In cases where the only waiter is cancelled, this PR will gracefully cancel the task and pop the key

Are there changes in behavior for the user?

Yes, a cancelled task will cancel any cached jobs that haven't completed and have no other waiters.

Related issue number

Checklist

• I think the code is well written
• Unit tests for the changes exist
• Documentation reflects the changes

[Dreamsorcerer]

Looks like a good idea. Should get a test added that reproduces the issue.

[BobTheBuidler]

I've added the tests and this is now ready for review.

But post-review, let's make sure we merge #688 before this one so we can start populating data in the codspeed dashboard

[codspeed-hq]

CodSpeed Performance Report

Merging #697 will improve performances by ×560

_{Comparing BobTheBuidler:patch-3 (928531f) with master (0654124)}

Summary

⚡ 6 improvements
✅ 57 untouched
⏩ 4 skipped¹

Benchmarks breakdown

	Benchmark	BASE	HEAD	Change
⚡	test_internal_task_done_callback_microbenchmark[cancelled-func-bounded]	3.5 ms	1.6 ms	×2.2
⚡	test_internal_task_done_callback_microbenchmark[cancelled-func-unbounded]	3.5 ms	1.6 ms	×2.2
⚡	test_internal_task_done_callback_microbenchmark[exception-func-bounded]	961.5 ms	1.7 ms	×560
⚡	test_internal_task_done_callback_microbenchmark[exception-func-unbounded]	963.7 ms	1.7 ms	×560
⚡	test_internal_task_done_callback_microbenchmark[finished-func-bounded]	3.9 ms	1.8 ms	×2.1
⚡	test_internal_task_done_callback_microbenchmark[finished-func-unbounded]	3.9 ms	1.8 ms	×2.1

Footnotes

1. 4 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

[BobTheBuidler]

Looks like there's one complication I didn't consider

If the cancelled waiter is the first waiter for a particular cache item, cancelling is easy because the waiter already has the task (it created the task)

But if any subsequent waiter is cancelled, that waiter did not create the task and does not have any reference to it. Only the CacheItem's fut.

is there any reason the tasks need to go in __tasks or is that just a common place for collection? Would it be acceptable to instead add a task field to the CacheItem dataclass and have the _task_done_callback remove it from there instead of using the current __tasks set?

That way, each CacheItem would be directly linked to its associated task and any waiter would have access to it for cancellation.

[Dreamsorcerer]

is there any reason the tasks need to go in __tasks or is that just a common place for collection?

Not sure. If the tests are passing, then another approach may be fine.

Would it be acceptable to instead add a task field to the CacheItem dataclass and have the _task_done_callback remove it from there instead of using the current __tasks set?

I guess as long as we don't end up with circular references or anything that might causes memory leaks, then it might work.

[BobTheBuidler]

@Dreamsorcerer I finally found some time to finish this up and add some test cases, not sure if this needs to be documented as it seems like what should be the expected behavior. Lmk what you think!

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.28%. Comparing base (0654124) to head (928531f).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #697      +/-   ##
==========================================
+ Coverage   97.22%   97.28%   +0.06%     
==========================================
  Files          12       13       +1     
  Lines         793      811      +18     
  Branches       41       44       +3     
==========================================
+ Hits          771      789      +18     
  Misses         20       20              
  Partials        2        2              

Flag	Coverage Δ
unit	95.93% <100.00%> (+0.09%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[BobTheBuidler]

I'm seeing a notable decrease in memory usage on some of my longer-running scripts and services!

[Dreamsorcerer]

Sorry about the delay. Looks all good to me.