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

Darwin kqueue #211

Closed
wants to merge 3 commits into from
Closed

Darwin kqueue #211

wants to merge 3 commits into from

Conversation

ioquatix
Copy link
Member

@ioquatix ioquatix commented Jul 7, 2019

No description provided.

@ioquatix ioquatix force-pushed the darwin-kqueue branch 2 times, most recently from 847363c to c78350f Compare July 7, 2019 01:13
@ioquatix
Copy link
Member Author

ioquatix commented Jul 7, 2019

@tarcieri Running the sample code from https://daniel.haxx.se/blog/2016/10/11/poll-on-mac-10-12-is-broken/ seems to indicate that at least poll is working on 10.14 - and I've enabled this in the PR and run the entire async-* test suite and it all seems to pass. On 10.14.5.

Given that nio4r breaking on macOS isn't probably going to destroy any production environments, maybe this is an acceptable change. What do you think?

@ioquatix
Copy link
Member Author

ioquatix commented Jul 7, 2019

[2] pry(main)> NIO::Selector.new.backend
=> :poll

Need to make it use kqueue by default.

@tarcieri
Copy link
Contributor

tarcieri commented Jul 7, 2019

@ioquatix I know poll(2) seemed to be working for awhile then regressed on macOS 10.12 so the platform is not exactly known for getting these sorts of APIs right, especially compared to the implementations on e.g. FreeBSD (which also had plenty of their own brokenness at first but eventually got better).

Otherwise, I’d say check if there have been any upstream libev changes around this if you haven’t already.

@ioquatix
Copy link
Member Author

ioquatix commented Jul 7, 2019

I recently updated nio4r to the latest libev release. I also checked the example source code from the blog post you linked to and it seems to pass at least on macOS 14.x - and according to those sources it was fixed on 10.12.2

I'll do some more investigations, but I think it should be safe (ish). As you state, macOS has a pretty shitty track record, so good to be cautious, but I think I have a pretty big test suite now, testing lots of fairly real-world situations.

@ioquatix ioquatix force-pushed the darwin-kqueue branch 2 times, most recently from 1de702b to e1f0fa7 Compare July 7, 2019 08:11
@ioquatix
Copy link
Member Author

ioquatix commented Jul 7, 2019

I decided to be a bit more conservative and only enable it for 10.14+ because that's what I can test easily.

@ioquatix
Copy link
Member Author

ioquatix commented Jul 7, 2019

I've merged it by hand.

@ioquatix ioquatix closed this Jul 7, 2019
@ioquatix ioquatix added this to the 2.4 milestone Jul 7, 2019
@ioquatix ioquatix self-assigned this Jul 7, 2019
@tarcieri tarcieri deleted the darwin-kqueue branch July 7, 2019 13:42
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.

2 participants