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

The manager's Close function is prone to race condition #17

Open
CermakM opened this issue Mar 21, 2024 · 0 comments
Open

The manager's Close function is prone to race condition #17

CermakM opened this issue Mar 21, 2024 · 0 comments
Labels
bug Something isn't working
Milestone

Comments

@CermakM
Copy link

CermakM commented Mar 21, 2024

Describe the bug
When calling the RunTask with a long running task def after Close, the task gets cancelled.

To Reproduce
Use the samples/slog example and add another manager.Close() call after the existing Close call and immediately after add manager.RunTask(ctx, def). The task is executed and killed after a moment.

Expected behavior
One possible solution is to forbid execution of tasks after the manager has been closed (this is a similar philosophy as for other Go Close methods, for example closing a request's body (io.ReadCloser) also does not allow to read from the body anymore (closed socket). I would imagine a lot of people will expect such behavior from a Close method.

If you don't want this kind of behavior, I would consider renaming the method to Stop instead and resolve the race condition.

@CermakM CermakM added the bug Something isn't working label Mar 21, 2024
@CermakM CermakM added this to the v0.1 milestone Mar 21, 2024
@robertrossmann robertrossmann modified the milestones: v0.1, v0.2 Apr 29, 2024
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

No branches or pull requests

2 participants