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

Fixes #1196 - Wait for redis listener to stop on Subscriber dtor #1201

Closed
wants to merge 3 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@etcimon
Contributor

etcimon commented Jul 28, 2015

The task that owns RedisSubscriber will now block if it must go out of scope, waiting for ..something.. to stop the listener on its object.

This also fixes a compiler issue I had on 2.068 with std.string.indexOf

fromStringz takes a slice and uses strlen: https://github.com/D-Programming-Language/phobos/blob/b34ff324f0bfe539a12797f0b29359681c8c888b/std/string.d#L207

}
// Task will block until the listener is finished
void waitForStop() {

This comment has been minimized.

@s-ludwig

s-ludwig Oct 10, 2015

Member

Since this is not allowed to be called concurrently, it should be private.

@s-ludwig

s-ludwig Oct 10, 2015

Member

Since this is not allowed to be called concurrently, it should be private.

@s-ludwig

This comment has been minimized.

Show comment
Hide comment
@s-ludwig

s-ludwig Oct 10, 2015

Member

I'd ideally turn around the logic and instead of waiting to stop in the destructor, keep a reference within the listener task, so that the destructor isn't run at all before everything is stopped. But for now this fix should work.

Member

s-ludwig commented Oct 10, 2015

I'd ideally turn around the logic and instead of waiting to stop in the destructor, keep a reference within the listener task, so that the destructor isn't run at all before everything is stopped. But for now this fix should work.

@WebFreak001

This comment has been minimized.

Show comment
Hide comment
@WebFreak001

WebFreak001 Oct 31, 2017

Contributor

@s-ludwig @etcimon status? merge or close?

Contributor

WebFreak001 commented Oct 31, 2017

@s-ludwig @etcimon status? merge or close?

s-ludwig added a commit that referenced this pull request Nov 5, 2017

s-ludwig added a commit that referenced this pull request Nov 5, 2017

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