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

Fix fibers::asio::round_robin's notify() method. #101

Closed
wants to merge 1 commit into from

Conversation

xaqq
Copy link
Contributor

@xaqq xaqq commented Nov 24, 2016

The notify() method of fibers::asio::round_robin doesn't give
a chance to pending fibers to run.

See #100 for more detail.

@olk
Copy link
Member

olk commented Nov 24, 2016

Implementation of has already changed, see branch develop

@olk olk closed this Nov 24, 2016
@xaqq
Copy link
Contributor Author

xaqq commented Nov 24, 2016

In develop, notify() simply set the timer to expires, and it seems its not enough.

@olk
Copy link
Member

olk commented Nov 24, 2016

you are right - Nat and I tried to get asio::round_robin working with multiple threads running io_service::run() of one instance of io_service. Unfortunately io_service it didn't work and not all changes were rolled back.
I'll take you example and add it to the library too - could you tell me your Name?

@olk olk reopened this Nov 24, 2016
@olk
Copy link
Member

olk commented Nov 24, 2016

merge must be into branch develop first - please change your pull request or tell me if I should do it (format-patch etc).

@xaqq xaqq changed the base branch from master to develop November 24, 2016 17:07
@xaqq
Copy link
Contributor Author

xaqq commented Nov 24, 2016

I changed the base on github, but it looks like it did something ugly. Feel free to close the PR and cherry-pick the commit or something if that keeps history cleaner.

Bear in mind that while this change seems to positively affect the example from #100 I think I'm still observing suspicious behavior when running my real program through valgrind. (with the use case being 2 io_service on 2 thread).

My name's Arnaud Kapp.

The notify() method of fibers::asio::round_robin doesn't give
a chance to pending fibers to run.

See boostorg#100 for more detail.
@olk
Copy link
Member

olk commented Nov 24, 2016

Arnaud, regarding to valgrind please apply valgrind=on at cmd-line.
(see http://www.boost.org/doc/libs/1_62_0/libs/context/doc/html/context/stack/valgrind.html)

@olk olk closed this Nov 24, 2016
@olk
Copy link
Member

olk commented Nov 24, 2016

pushed to branch develop

@xaqq
Copy link
Contributor Author

xaqq commented Nov 25, 2016

Thanks.

WRT valgrind, I already use valgrind=on. The issue is not invalid memory access / "client switching stack". Rather it looks like spinning. I can't reproduce w/o valgrind so it's a bit weird.
I'll report back if I find something.

@xaqq xaqq mentioned this pull request Nov 29, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants