Fix uv__io_stop assertion failures

[mdlavin]

When running an application that uses many different ZooKeeper connections, and experiences many 'operation timeout' errors, I would frequently see this failure:

Assertion failed: (loop->watchers[w->fd] == w), function uv__io_stop, file ../deps/uv/src/unix/core.c, line 701.
Abort trap: 6

After some debugging here is what I saw:

• An initial call to zookeeper_interest returns a file handle, let's say fd=100
• The code starts watching that file handle with uv_poll_init / uv_poll_start
• Later, a call to zookeeper_interest with fd=100 fails with operation timeout. That triggers the if (rc) block, which does not stop watching the file handle
• After some time a new call to zookeeper_interest returns the same fd=100 handle again, for a different client.
• Eventually, uv_* is called with fd=100 and libuv fails with the assertion above.

The changes I've attached will always call uv_poll_stop, even in the case of an error, so that the file handle will be stop being watched. After my changes, I can run my many-client, many-timeout test without any assertion failures.

[saghul]

You can safely call uv_poll_stop directly, it wont do anything if the handle is not active.

[mdlavin]

@yfinkelstein any chance this could get merged in? I've been running with my forked version since I posted this and I haven't yet seen any trouble. I'd love to get back to using a copy from npmjs.org.

[sh1mmer]

@yfinkelstein +1 on this

[mdlavin]

@yfinkelstein any chance that this could be merged in? I've been maintaining a fork of node-zookeeper with my pending pull requests and I'd love to get back into sync with the project.

[mdlavin]

@kuebk it looks like you might have access to mering into this repo. Is there any chance I can get your help merging a couple of fixes into this project?

[yfinkelstein]

@mdlavin You can always send a pull request in. I'm not actively working on the project, if the pull request is auto-resolvable I can review and accept. Thanks!

[yfinkelstein]

@mdlavin Just accepted 2 of your pull requests

[mdlavin]

Thanks for merging the other fixes. If there is a problem with this
change, can you let me know?

I am part of a group that is actively using this project, is there any
chance I could get access to merge and publish to npm? I'd love to keep
this project alive and can help merging some of the other changes too.
On Dec 27, 2014 12:46 PM, "Yuri Finkelstein" [email protected]
wrote:

@mdlavin https://github.com/mdlavin Just accepted 2 of your pull
requests

—
Reply to this email directly or view it on GitHub
#101 (comment)
.

[yfinkelstein]

Matt,
I just added you as a collaborator. Feel free to check in directly and merge the outstanding pull request. The reason I have not done it myself is that this one requires fairly extensive testing and I don't have time for it myself. Since you say you use this code in production I feel I can trust you with making sure the change is fully tested.
Thanks for your contributions! I will also add your name to the list of contributors in readme.