Add lock for thread-safe access to winterm

[polys]

When the output stream is wrapped, print(...) is no longer an atomic operation. With multiple threads writing to the console, the text and colours sometimes get a bit scrambled.

This PR wraps the write method with a lock to make it atomic.

I have working code that consistently reproduces this and also verifies that the lock resolves the issue but I'm unable to share. I can try to come up with a simpler example that I can share, if needed.

[Delgan]

Hi. Shouldn't the caller be in charge of protecting and synchronizing concurrent write rather than incorporate it into colorama? Is the atomicity of print() "officially" documented somewhere?

[polys]

I'm not sure if the atomicity of print is documented to be honest; with Python's GIL, people take these things for granted.

My reasoning was that, since this is a 'compatibility' wrapper, it should try to emulate the observed behaviour of the function being wrapped as close as possible.

[wiggin15]

Locking the write calls incurs a performance penalty that I'm not sure we should apply on our level - most applications don't have any problems with thread safety.
I suspect the actual issue here has to do with the "flush" calls colorama calls after each write causing different behaviors between regular prints and the colorama wrapper, and not the atomicity of the write operation (can you please check if removing the "flush" calls from ansitowin32.py fixes your issue?).

As @Delgan suggested, maybe when using colorama and having these kinds of threading problems, the caller should apply the locks to synchronize the writes.

[polys]

I did think of flush and tried quite a few things before resorting to locks.

If each thread called print multiple times and somehow expected the console output to be correct then that's wrong - I agree that in that case, it is the caller's problem to synchronize the writes.

What I'm trying to address here is different though... when wrapping the stream, colorama essentially breaks up what was a single string from a single call to print, and writes the parts separately to the underlying stream, interleaved with the necessary calls to Win32 APIs to style the console. To achieve that, colorama uses a cursor to track the offset and in doing so, it assumes that the underlying stream isn't changing between calls. That's a fair assumption for Python code because of the GIL, but it's not for the native code executed by the Win32 API calls.

Also, the performance hit from the lock only applies to Windows, and only when wrapping is enabled. I'd be surprised if a client so concerned with performance would be using colorama with wrapping enabled while writing to the console from multiple threads, and do all that in Python.

All that said, this PR was only an attempt to save your average scripter from surprises now that futures and concurrency are getting easier and more popular in Python. I'm happy to close the PR if people think it's a bad idea... resorting to locks isn't exactly my preference.

The only reason I witnessed this behaviour is because I put together a not-so-amazing script in 10 minutes or so, just to batch-execute some HTTP requests... to speed things up, I replaced a simple 2-line loop with ThreadPoolExecutor but forgot a print I was using for debugging in the thread function 🤓

[tartley]

Hey. FYI, Yesterday I created a PR to test releases before we push them to PyPI. When that is merged, I'll be more confident about resuming merges and releases. I'll try to look at this PR soon. Thank you for creating it!

[tartley]

Apologies for being AWOL for years, but I'm rounding up PRs that might need merging into Colorama. If you're still keen to champion this, @polys, let me know. It seems useful to me. I don't tend to allow myself to get trapped in threading issues if I can possibly avoid it, so I shan't pretend to be an expert. But also, I'm not terrifically worried about performance overhead until someone demonstrates it's an issue.

Having said that, I'd be much more keen to merge this if there was a demonstration of the problem that we could use to justify it, rather than voodoo "I heard it worked on someone's machine once". :-) If that demonstration could be baked in a test, even better. I understand that isn't trivial to do, so would be willing to settle for a human-run demo instead (which I'm not proposing to merge)