DreamLab / memoize

Caching library for asynchronous Python applications.
https://memoize.readthedocs.io
Apache License 2.0
69 stars 8 forks source link

Wrapping a function that raises a BaseException (not an Exception) can indefinitely wedge all future calls using that argument list #35

Closed charles-dyfis-net closed 2 weeks ago

charles-dyfis-net commented 2 months ago

Because refresh calls mark_being_updated() before it invokes value_future_provider() or await value_future, any failure in those calls that doesn't go through the existing exception handlers never calls mark_update_aborted(), and thus leaves a future that will never be populated in the status tracker.

I have a fix for this (incorporated in https://github.com/charles-dyfis-net/memoize/tree/local), but it lives in a tree that makes other heavy-handed changes which may or may not be acceptable upstream.

charles-dyfis-net commented 1 month ago

Note that this can happen even for code that doesn't ever explicitly raise a non-Exception throwable: CancelledError in recent versions of Python (since at least 3.8) inherits directly from BaseException; see this change discussed in https://bugs.python.org/issue32528

Thus, for any web framework that cancels coprocesses generating for pages where the client disconnects, memoize-wrapped functions can get wedged when using the upstream tree.

zmumi commented 2 weeks ago

Good point about CancelledError inheriting from BaseException since 3.8. Still other BaseExceptions like GeneratorExit, KeyboardInterrupt, or SystemExit (see https://docs.python.org/3/library/exceptions.html#exception-hierarchy) IMO should not be caught, as they are related to process termination and we should not disrupt that. So I'd proceed to explicitly catch CancelledError only

charles-dyfis-net commented 2 weeks ago

I no longer have a dog in this fight -- replaced py-memoize with something built to a narrower set of requirements -- but I'd generally tend to suggest catching all BaseException types and then re-throwing them; that way a parent process can choose to suppress a KeyboardInterrupt without wedging the state of a cached call that was underway.