Timeoutadding timeout to wmi connection initialization and query #3
Conversation
| raise_if_failed(result) | ||
| rescue WIN32OLERuntimeError => native_exception | ||
| raise WmiException.new(native_exception, :ExecQuery, @namespace, wql_query, diagnostic_class_name) | ||
| Timeout::timeout(@timeout, WmiTimeoutException) do |
There was a problem hiding this comment.
Hmm, so what does this do -- kill the thread?
There was a problem hiding this comment.
yup. it throws the 2nd parameter.
There was a problem hiding this comment.
So to clarify, Timeout creates a new thread, and then kills it if things take too long. We should definitely add some comments to clarify that so everyone can understand the impact on COM / OS thread state.
There was a problem hiding this comment.
Looks like side effects of hitting timeout can be severe - http://www.mikeperham.com/2015/05/08/timeout-rubys-most-dangerous-api/
There was a problem hiding this comment.
interesting. I know we use it in several places inside chef and chef-provisioning (thats where I learned about it) will investigate
|
I made the default behavior the same as it is today and now you have to pass a timeout to the initializer. So once this releases, I'll update the calls from ohai. I'm hoping customers suffering from hung wmi calls can just update their ohai gem without updating the chef client. |
|
It's starting to look like maybe ExecQueryAsync https://msdn.microsoft.com/en-us/library/aa392108(v=vs.85).aspx is the way to go. Or perhaps something in the application itself that doesn't kill any threads but simply exits the process. @chefsalim, in this case, I think using timeout is ok IF you exit the process when it times out. That's a problem with putting the timeout in this gem, so maybe the right thing is to document the behavior that if you specify a timeout, you must exit the process. |
No description provided.