dbrattli / Expression

Pragmatic functional programming for Python inspired by F#
https://expression.readthedocs.io
MIT License
421 stars 30 forks source link

Fishy locking in CancellationSource #94

Closed ytimenkov closed 1 year ago

ytimenkov commented 1 year ago

Describe the bug Usage of _lock in https://github.com/cognitedata/Expression/blob/main/expression/system/cancellation.py doesn't look correct.

I was looking through files and got puzzled by how CancellationTokenSource.dispose() is implemented.

Apparently it tries to read _is_disposed under the lock but on the next like assigns it without lock. It also calls listeners without lock.

On the other hand register_internal seems to do the opposite: it reads _is_disposed without lock and accesses _listeners inside a critical section.

To be strict all access to variables should be protected and protected consistently. E.g. if it's dangerous to call a callback under the lock - a copy (slice) of _listeners could be made (in the same critical section with checking for _is_disposed).

dbrattli commented 1 year ago

@ytimenkov Thanks for posting this issue. It indeed looks like a bug.