Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Race condition: CLPLogLevelTimeout and the logging handler may concurrently write to the underlying output stream. #55

Open
LinZhihao-723 opened this issue Feb 6, 2025 · 0 comments
Labels
bug Something isn't working

Comments

@LinZhihao-723
Copy link
Member

Bug

In the current implementation, both CLPLogLevelTimeout and CLP logging handlers could write to the underlying stream concurrently without locking the stream. The write operation involves the stream's write and flush methods. The potential types of streams are Python built-in byte IO stream and zstdandard's streaming compressor writer.

This is problematic because the underlying write operation may release the GIL. For example:

The bug was first triggered in a dev branch when supporting serializing key-value pair log events into CLP key-value pair IR streams. Both the logging handler and the timeout handler attempt to write into the zstd stream, causing a race condition that leads to a segfault. It's been verified that the bug will be resolved by removing the above liens from Python and zstd's source code, or by adding a lock to ensure mutually exclusive access to the stream.

To conclude, our engineering decision through the offline discussion is to add a lock for mutual exclusion.

Python clp-logging version

53242ec

Environment

This bug should exist in any platform/environment we currently support.

Reproduction steps

There are no specific steps to reproduce this bug since it is triggered in a dev branch with some code that won't be merged.
However, we've collected enough evidence that race conditions could happen and we should lock the timeout handler properly.

@LinZhihao-723 LinZhihao-723 added the bug Something isn't working label Feb 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant