Better testing.

Author: samuel-williams-shopify

fixtures/async/container/a_container.rb

@@ -246,6 +246,91 @@ module Container
 					expect(container.statistics).to have_attributes(failures: be > 0)
 				end
 			end
+			
+			with "broken children" do
+				it "can handle children that ignore termination with SIGKILL fallback" do
+					# Test behavior that works for both processes (signals) and threads (exceptions)
+					container.spawn(restart: false) do |instance|
+						instance.ready!
+						
+						# Ignore termination attempts in a way appropriate to the container type
+						if container.class.multiprocess?
+							# For multiprocess containers - ignore signals
+							Signal.trap(:INT) {}
+							Signal.trap(:TERM) {}
+							while true
+								sleep(0.1)
+							end
+						else
+							# For threaded containers - ignore exceptions
+							while true
+								begin
+									sleep(0.1)
+								rescue Async::Container::Interrupt, Async::Container::Terminate
+									# Ignore termination attempts
+								end
+							end
+						end
+					end
+					
+					container.wait_until_ready
+					
+					# Try to stop with a very short timeout to force escalation
+					start_time = Time.now
+					container.stop(0.1) # Very short timeout
+					end_time = Time.now
+					
+					# Should stop successfully via SIGKILL/thread termination
+					expect(container.size).to be == 0
+					
+					# Should not hang - escalation should work
+					expect(end_time - start_time).to be < 2.0
+				end
+				
+				it "can handle unresponsive children that close pipes but don't exit" do
+					container.spawn(restart: false) do |instance|
+						instance.ready!
+						
+						# Close communication pipe to simulate hung process:
+						begin
+							if instance.respond_to?(:out)
+								instance.out.close if instance.out && !instance.out.closed?
+							end
+						rescue
+							# Ignore close errors.
+						end
+						
+						# Become unresponsive:
+						if container.class.multiprocess?
+							# For multiprocess containers - ignore signals and close file descriptors:
+							Signal.trap(:INT) {}
+							Signal.trap(:TERM) {}
+							(4..256).each do |fd|
+								begin
+									IO.for_fd(fd).close
+								rescue
+									# Ignore errors
+								end
+							end
+							loop {} # Tight loop
+						else
+							# For threaded containers - just become unresponsive
+							loop {} # Tight loop, no exception handling
+						end
+					end
+					
+					container.wait_until_ready
+					
+					# Should not hang even with unresponsive children
+					start_time = Time.now
+					container.stop(1.0)
+					end_time = Time.now
+					
+					expect(container.size).to be == 0
+					# Should complete reasonably quickly via hang prevention
+					expect(end_time - start_time).to be < 5.0
+				end
+			end
 		end
 	end
 end

lib/async/container/group.rb

@@ -165,7 +165,7 @@ def stop(graceful = true, interrupt_timeout: 1, terminate_timeout: 1)
 				end
 
 				Console.debug(self, "Stopping all processes...", interrupt_timeout: interrupt_timeout, terminate_timeout: terminate_timeout)
-
+				
 				# If a timeout is specified, interrupt the children first:
 				if interrupt_timeout
 					clock = Async::Clock.start

test/async/container/forked.rb

@@ -68,77 +68,4 @@
 	it "should be multiprocess" do
 		expect(subject).to be(:multiprocess?)
 	end
-		
-	it "can handle children that ignore SIGTERM with SIGKILL fallback" do
-		# Create a child that ignores SIGINT and SIGTERM
-		container.spawn(restart: false) do |instance|
-			# Trap both SIGINT and SIGTERM to ignore them (like the example)
-			Signal.trap(:INT) {}
-			Signal.trap(:TERM) {}
-				
-			instance.ready!
-				
-			# Infinite loop that can only be stopped by SIGKILL
-			while true
-				sleep(0.1)
-			end
-		end
-			
-		container.wait_until_ready
-			
-		# Try to stop with a very short timeout
-		# This should first try SIGINT, then SIGTERM, then fall back to SIGKILL
-		start_time = Time.now
-		container.stop(0.1) # 100ms timeout - very short
-		end_time = Time.now
-			
-		# The container should stop successfully even though the child ignored signals
-		expect(container.size).to be == 0
-			
-		# It should not take too long (should not hang waiting for SIGTERM)
-		# Allow some buffer time for the SIGKILL fallback mechanism
-		expect(end_time - start_time).to be < 2.0
-	end
-		
-	it "can handle unresponsive children that close pipes but don't exit" do
-		# Simulate a production hang scenario where a child closes file descriptors
-		# but doesn't actually exit, becoming unresponsive
-		container.spawn(restart: false) do |instance|
-			# Ignore all signals
-			Signal.trap(:INT) {}
-			Signal.trap(:TERM) {}
-				
-			instance.ready!
-				
-			# Close all file descriptors above 3 (like the production hang scenario)
-			# This will close the notify pipe, making the parent think we've "exited"
-			(4..256).each do |fd|
-				begin
-					IO.for_fd(fd).close
-				rescue
-					# Ignore errors for non-existent file descriptors
-				end
-			end
-				
-			# Now become unresponsive (infinite loop without yielding)
-			while true
-				# Tight loop without sleep - process is unresponsive but still alive
-			end
-		end
-			
-		container.wait_until_ready
-			
-		# This should not hang - even with unresponsive processes, stop should work
-		start_time = Time.now
-		container.stop(2.0) # Give it a reasonable timeout for testing
-		end_time = Time.now
-			
-		# Container should stop successfully
-		expect(container.size).to be == 0
-			
-		# Should complete within the child's individual timeout + buffer (30s + 5s)
-		# The process is so unresponsive it needs the individual Child timeout to kill it
-		# This proves the hang prevention works - without it, this would hang forever
-		expect(end_time - start_time).to be < 35.0
-	end
 end if Async::Container.fork?