Skip to content

fix(go-grpc-server): always close resultChan - #3732

Closed
mudler wants to merge 1 commit into
masterfrom
fix/closedChannel
Closed

fix(go-grpc-server): always close resultChan#3732
mudler wants to merge 1 commit into
masterfrom
fix/closedChannel

Conversation

@mudler

@mudler mudler commented Oct 4, 2024

Copy link
Copy Markdown
Owner

Description

By not closing the channel, if a server not implementing PredictStream receives a client call would hang indefinetly as would wait for resultChan to be consumed.

If the prediction stream returns we close the channel now and we wait for the goroutine to finish.

Notes for Reviewers

The issue can be reproduced easily: install a whisper model, and try to chat it via the WebUI. Any call to other backend is now locked as LocalAI tries to shutdown the whisper model and fail as it still hanging, waiting for the previous PredictStream request to complete.

Only golang-based models are affected (e.g. tts with rhaspy and whisper)

Signed commits

  • Yes, I signed my commits.

@mudler mudler added the bug Something isn't working label Oct 4, 2024
By not closing the channel, if a server not implementing PredictStream
receives a client call would hang indefinetly as would wait for
resultChan to be consumed.

If the prediction stream returns we close the channel now and we wait
for the goroutine to finish.

Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
@mudler
mudler force-pushed the fix/closedChannel branch from fce2031 to 8311089 Compare October 4, 2024 22:08
@netlify

netlify Bot commented Oct 4, 2024

Copy link
Copy Markdown

Deploy Preview for localai ready!

Name Link
🔨 Latest commit fce2031
🔍 Latest deploy log https://app.netlify.com/sites/localai/deploys/670067353ca14c00080bb791
😎 Deploy Preview https://deploy-preview-3732--localai.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@netlify

netlify Bot commented Oct 4, 2024

Copy link
Copy Markdown

Deploy Preview for localai ready!

Name Link
🔨 Latest commit 8311089
🔍 Latest deploy log https://app.netlify.com/sites/localai/deploys/670067462e64160008519c4d
😎 Deploy Preview https://deploy-preview-3732--localai.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

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

Successfully merging this pull request may close these issues.

1 participant