Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Potential go routine on blocking functions #623

Closed
davidrenne opened this issue Sep 15, 2023 · 4 comments
Closed

Potential go routine on blocking functions #623

davidrenne opened this issue Sep 15, 2023 · 4 comments
Assignees

Comments

@davidrenne
Copy link

In the case of an accidental blocked thread where the calling function has incorrect or bad code:

https://github.com/ably/ably-go/blob/main/ably/realtime_channel.go#L512

An entire set of messages will be blocked forever. Do you think you can add a go routine for this to prevent this? I realize every 1 payload will be a go routine, but perhaps you can call the function callback and timeout to kill the go routine somehow if it doesnt return in a certain number of seconds?

It wasnt inherently obvious that these callbacks are synchronous and thus if I have 1 payload which incorrectly hangs a channel without a select timeout, my entire servers pub sub goes down. Now I have added selects and timeouts if the channel is gone so that these things clear. So hopefully it solves my problem overall, but I saw this code and wanted to have a discussion about strategies or documentation around a blocking function call that can ruin an entire servers pubsub.

@sync-by-unito
Copy link

sync-by-unito bot commented Sep 15, 2023

➤ Automation for Jira commented:

The link to the corresponding Jira issue is https://ably.atlassian.net/browse/SDK-3855

@sacOO7
Copy link
Collaborator

sacOO7 commented Sep 15, 2023

Hi @davidrenne we have intentionally kept public methods as blocking in nature.
You might like to read => https://github.com/ably/ably-go/blob/main/UPDATING.md#asynchronous-operations-and-the-context-concurrency-pattern
If you don't want to block on the method call, you can externally cancel the context.
Let me know if more information is needed.

@sacOO7
Copy link
Collaborator

sacOO7 commented Sep 15, 2023

This also means, that if you don't want to wait till the call to return, you have to implement your own mechanism to handle success/failure of the operation asynchronously.
I won't recommend spawning new goroutine for every method call, because creating threads still is an expensive operation and should be managed as a threadpool ( pool of limited threads ).
You might like to check https://betterprogramming.pub/building-and-testing-a-worker-pool-in-go-bce4c6da4431

@sacOO7 sacOO7 self-assigned this Oct 12, 2023
@sacOO7
Copy link
Collaborator

sacOO7 commented Oct 12, 2023

@davidrenne let us know if you have any updates on this, otherwise, we will close the issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants