Fix audio mlx server connect error #1057
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
| request_id = uuid.uuid4() | ||
| params["request_id"] = str(request_id) | ||
| output = await worker.generate(params) | ||
| release_worker_semaphore() |
There was a problem hiding this comment.
This code is not thread safe, there is no guarantee that release_worker_semaphore() will ever get called. I would highly recommend using a finally: clause.
There was a problem hiding this comment.
Just trying to understand if this also impacts other things. What do you mean by not being thread safe? is it because of the try catch or something else
There was a problem hiding this comment.
It's difficult to explain thoroughly without getting into the complexities of thread management, but basically if worker.generate throws an exception, then release_worker_semaphore() will never get called, which is problematic. The fix should be easy though, you should be able to do something like
finally:
release_worker_semaphore()
to ensure that this line does get called.
It's really easy to write code that isn't thread-safe so definitely keep an eye out for stuff like this.
| lang_code = params.get("lang_code", None) | ||
| stream = params.get("stream", False) | ||
|
|
||
| experiment_dir = get_experiments_dir() |
There was a problem hiding this comment.
Just a suggestion but I think this doesnt work because the lab context getting set when this file runs is not passed on to the uvicorn app that runs from this. I do set a lab context so the get_experiments_dir should've worked if it was running directly in this file. Do you think a better approach would be passing in experiments dir through the main() function by calling this function there? Or does that not work?
I'm just concerned if plugin_harness did not set the context correctly for this case and was wanting to get to the core of the issue
No description provided.