WinRb / WinRM

Ruby library for Windows Remote Management
Apache License 2.0
412 stars 117 forks source link

Added fix for thread error #342

Open nikhil2611 opened 7 months ago

nikhil2611 commented 7 months ago

Fixed the below thread error coming up on windows nodes on the execution of the kitchen verify and knife commands.

Test Summary: 0 successful, 0 failures, 1 skipped Finished verifying <default-windows-2022-base1> (0m18.89s). -----> Test Kitchen is finished. (0m35.20s) /opt/chef-workstation/bin/kitchen: warning: Exception in finalizer #<Proc:0x000000011bf58a18 /opt/chef-workstation/embedded/lib/ruby/gems/3.1.0/gems/winrm-2.3.6/lib/winrm/shells/power_shell.rb:33> /opt/chef-workstation/embedded/lib/ruby/gems/3.1.0/gems/logging-2.3.1/lib/logging/diagnostic_context.rb:471:in new': can't alloc thread (ThreadError)
from /opt/chef-workstation/embedded/lib/ruby/gems/3.1.0/gems/logging-2.3.1/lib/logging/diagnostic_context.rb:471:in create_with_logging_context' from /opt/chef-workstation/embedded/lib/ruby/gems/3.1.0/gems/logging-2.3.1/lib/logging/diagnostic_context.rb:436:in new'
from /opt/chef-workstation/embedded/lib/ruby/gems/3.1.0/gems/timeout-0.3.2/lib/timeout.rb:101:in create_timeout_thread' from /opt/chef-workstation/embedded/lib/ruby/gems/3.1.0/gems/timeout-0.3.2/lib/timeout.rb:134:in block in ensure_timeout_thread_created'
from /opt/chef-workstation/embedded/lib/ruby/gems/3.1.0/gems/timeout-0.3.2/lib/timeout.rb:132:in synchronize' from /opt/chef-workstation/embedded/lib/ruby/gems/3.1.0/gems/timeout-0.3.2/lib/timeout.rb:132:in ensure_timeout_thread_created'
from /opt/chef-workstation/embedded/lib/ruby/gems/3.1.0/gems/timeout-0.3.2/lib/timeout.rb:181:in timeout' from /opt/chef-workstation/embedded/lib/ruby/gems/3.1.0/gems/httpclient-2.8.3/lib/httpclient/session.rb:515:in query'
from /opt/chef-workstation/embedded/lib/ruby/gems/3.1.0/gems/httpclient-2.8.3/lib/httpclient/session.rb:177:in query' from /opt/chef-workstation/embedded/lib/ruby/gems/3.1.0/gems/httpclient-2.8.3/lib/httpclient.rb:1242:in do_get_block'
from /opt/chef-workstation/embedded/lib/ruby/gems/3.1.0/gems/httpclient-2.8.3/lib/httpclient.rb:1019:in block in do_request' from /opt/chef-workstation/embedded/lib/ruby/gems/3.1.0/gems/httpclient-2.8.3/lib/httpclient.rb:1133:in protect_keep_alive_disconnected'
from /opt/chef-workstation/embedded/lib/ruby/gems/3.1.0/gems/httpclient-2.8.3/lib/httpclient.rb:1014:in do_request' from /opt/chef-workstation/embedded/lib/ruby/gems/3.1.0/gems/httpclient-2.8.3/lib/httpclient.rb:856:in request'
from /opt/chef-workstation/embedded/lib/ruby/gems/3.1.0/gems/httpclient-2.8.3/lib/httpclient.rb:765:in post' from /opt/chef-workstation/embedded/lib/ruby/gems/3.1.0/gems/winrm-2.3.6/lib/winrm/http/transport.rb:176:in send_request'
from /opt/chef-workstation/embedded/lib/ruby/gems/3.1.0/gems/winrm-2.3.6/lib/winrm/shells/power_shell.rb:42:in close_shell' from /opt/chef-workstation/embedded/lib/ruby/gems/3.1.0/gems/winrm-2.3.6/lib/winrm/shells/power_shell.rb:33:in block in finalize'`

Issue - https://github.com/chef/chef-workstation/issues/3073

Note- This warning is coming up in the stdout while executing kitchen verify command, though it is not effecting kitchen verify command execution. We have added a patch to fix this warning if the thread is in dead or sleep state.

mwrock commented 7 months ago

Thinking this through, I think the waking up if asleep is perfectly safe, but returning false if the thread is dead may not be the right approach here. We don't want to silently continue if we cannot send a CloseShell message to the server. It is possible that one could end up with lots of orphaned shell processes on the server and we should have some way of communicating to the user that we could not close the shell.

tpowell-progress commented 7 months ago

Thinking this through, I think the waking up if asleep is perfectly safe, but returning false if the thread is dead may not be the right approach here. We don't want to silently continue if we cannot send a CloseShell message to the server. It is possible that one could end up with lots of orphaned shell processes on the server and we should have some way of communicating to the user that we could not close the shell.

@mwrock I think the dead thread limited options, maybe an exception can be raised instead?

mwrock commented 7 months ago

Yeah @tpowell-progress an exception could be raised if the thread is dead leading to a clearer exception message. I was looking at this some more today and I think this could also be remedied if kitchen or kitchen-inspec or inspec or whoever is creating the shell, explicitly closes it when done. This way you could be assured the thread is live and it is not left to the finalizer after kitchen exits. It looks like kitchen is good about doing that so I suspect this is somewhere in inspec.