-
Notifications
You must be signed in to change notification settings - Fork 111
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
Node crashes if timeout (rc: -7) is thrown during init() #262
Comments
Thank you for reporting this @sirmonin! |
Could be related to #238 |
Hi @sirmonin! To find out what could be wrong:
|
@DavidVujic, thanks for a quick reply! node-zookeeper: Tested both on 4.6.0 and 4.7.1 |
Great, thanks! |
@DavidVujic It is also observed, that timeout with crashing happens while application is running under some load. |
Thanks! I'm planning to look at this later this week. Notes: As hinted in this StackOverflow post: https://stackoverflow.com/questions/47522854/main-src-unix-core-c117-uv-close-assertion-uv-is-closinghandle-failed If you, or if you know someone with C and/or C++ skills, please feel free to jump in and help 😄 It is much appreciated. |
Just a short update: I'm working on this issue and am trying different approaches on how to solve it. I also believe I can reproduce the assertion error using the code in the |
Hello again @sirmonin! I've been working on solving this issue, but cannot really find where and how to prevent the assertion error to happen. What I've found out is that it most likely is when watchers are attached. Earlier today I pushed new versions of the code in the And that has made a difference: The way I've been doing is: Before the changes in the example code, I got assertion error every time. Now, only randomly it seems I have never got the error you are describing (with short timeouts). Could you help me by sharing another code example, or if you can try the example code described above? Here's a diff of the changes I have made: https://github.com/yfinkelstein/node-zookeeper/pull/264/files |
Hi @DavidVujic! Thanks a lot for time and effort you've spent on this issue. My zookeeper connect method looks like this. Timeout is set to 5ms to simulate timeout error.
This produces error logs:
|
However, adding a setTimeout seemed to help a bit. App is no longer crashing and tries to reconnect.
|
Thanks @sirmonin! |
I'm closing this one. Please reopen if it still is an issue! |
@DavidVujic Issue still persists when server is under high load.
Node.js v14.16.1 |
Thank you for notifying. I'll reopen this one! |
@DavidVujic I tested under Node v16.14.2. It looks like yield:zookeeper_interest returned error: -7 - operation timeout no longer causes libuv to fail. Will keep you posted in case issue repeats itself again. However, timeout issue still persists (eventhough it doesn't crash node anymore)
Update: Node.js v14.19.1 doesn't crash as well. |
Describe the bug
If timeout is reached (rc: -7) during the first init() call, I receive the error below and app crashes with code: 139.
I am sure that app doesn't use Zookeeper instance until there is a 'connect' event (which is never received).
To Reproduce
.init()
or.connect()
with a timeout set to a tiny value. E.g. 5ms. (This will produce the same effect as on our server, where timeout is set to 10000ms.)Error Log
Expected behavior
Timeout doesn't crash node app and closes client connection, so I could recreate client instance and retry to connect.
Desktop (please complete the following information):
The text was updated successfully, but these errors were encountered: