bslatkin / effectivepython

Effective Python: Second Edition — Source Code and Errata for the Book
https://effectivepython.com
2.2k stars 710 forks source link

Item 63: "WriteThread" class implementation #104

Open dongbohu opened 2 years ago

dongbohu commented 2 years ago

In Item 63 (Mix Threads and Coroutines to Ease the Transition to asyncio), I don't think WriteThread needs to be a subclass of Thread, because the threading part is automatically handled by the default ThreadPoolExecutor that is associated with the main thread's event loop. See line 127: https://github.com/bslatkin/effectivepython/blob/4ae6f3141291ea137eb29a245bf889dbc8091713/example_code/item_63.py#L127

So lines 85-88: https://github.com/bslatkin/effectivepython/blob/4ae6f3141291ea137eb29a245bf889dbc8091713/example_code/item_63.py#L85-L88 can be simplified into:

class Writer():
    def __init__(self, output_path):
        self.output_path = output_path

(I changed the class name too.)

Of course line 127 should be change to:

await loop.run_in_executor(None, self.run)

and WriteThread in other places should be changed to Writer.

bslatkin commented 1 month ago

Thank you for the report!

For this basic example it's true that using just the executor might work.

But the key detail here is the call to asyncio.set_event_loop, which sets the event loop for the current running thread. If the worker thread is in a ThreadPoolExecutor then the event loop should be the same as the main event loop for the overall program. But this example is trying to create a separate event loop that's totally independent from it. If you set_event_loop inside the executor to another event loop, then you'll cause later tasks in the same thread pool to run on the wrong event loop, causing really weird behavior or completely stalled/deadlocked tasks.

I'll add some details to explain this more clearly.

dongbohu commented 1 month ago

Got it. Thank you for your reply!