search

Azure / azure-uamqp-python

AMQP 1.0 client library for Python
MIT License
56 stars 48 forks source link

Fix thread not started error #289

Closed Dreamsorcerer closed 2 years ago

Dreamsorcerer commented 2 years ago

We keep getting errors from our code that a thread has not started while trying to .join() it on line 285.

Most likely the thread failed to start, which throws a RuntimeError, then the azure library catches the error and calls the close() method. At this point, the thread has been assigned to the attribute, but has failed to start, which causes the code to call .join() and error.

Proposed solution is to reset the attribute to None when such an error occurs.

yunhaoling commented 2 years ago

hey @Dreamsorcerer , thanks for the PR, I think your fix should work.

btw, do you know why the thread failed to start -- what's the error information? because depending on the root cause, I think we could choose not to raise the error, instead, we just do a warning logging.

Dreamsorcerer commented 2 years ago

Either the pymongo and/or azure libraries seem to be using way too many threads, so we seem to keep hitting the limit and the OS won't allow any more threads to be created.

We get these errors in both libraries fairly frequently, so it must be one or the other that is using too much, as both of these libraries are not native asyncio, but async wrappers around a thread-based library.

Dreamsorcerer commented 2 years ago

Note that the calling azure code catches these exceptions and attempts to retry, so this would not be the place to suppress the exception.

yunhaoling commented 2 years ago

hey @Dreamsorcerer , apologize for missing the thread here.

I agreed with you that this error should not silently pass and should be raised as it will handled by the upper application as well as for the sake of non-breaking behavior.

however, I'm a bit concerned on doing it during open stage, and I think there's a simpler way to do it -- I have left a comment on your PR, please let me know your thoughts on it and would be happy o collaborate with you on your PR and get it merged.