[CI] Fix BackgroundResources double-cleanup crash by adding guard

[AndreasKaratzas]

Fixes regression after: #34730

BackgroundResources.__call__() crashes with AttributeError: 'BackgroundResources' object has no attribute 'output_queue_task' when invoked more than once. This happens because the cleanup path uses del self.output_queue_task which removes the attribute entirely, so a second call fails.

This is triggered in practice when the engine monitor thread detects a dead engine and calls shutdown(), followed by the caller (e.g. a test) also calling shutdown() explicitly. Both paths end up invoking self.resources().

• Add a _cleaned_up bool guard so __call__ subsequent calls are a no-op.
• Replace del self.output_queue_task / del self.stats_update_task with = None assignments to clear references without removing the attribute.
• Nil out engine_manager and coordinator after shutdown to prevent double-shutdown when MPClient.shutdown() calls engine_manager.shutdown(timeout=...) followed by self.resources().

cc @kenroche

[gemini-code-assist]

Code Review

This pull request addresses a crash caused by double-cleanup in BackgroundResources by introducing an idempotency guard. It also correctly replaces del with None assignments to prevent AttributeError on subsequent cleanup attempts. The changes are logical and directly fix the described issue. I've added one suggestion to make the idempotency guard thread-safe.

[gemini-code-assist]

The current idempotency check is not thread-safe. A race condition can occur where two threads both check self._cleaned_up before it's set to True, leading to the cleanup logic running twice. Using a threading.Lock ensures that the check-and-set operation is atomic, preventing this race.

I'm suggesting a change that introduces a lock to the BackgroundResources class and uses it within __call__ to safely manage the cleanup state. You'll need to add from threading import Lock and from dataclasses import field at the top of the file to apply this suggestion.

Suggested change

	if self._cleaned_up:
	return
	self._cleaned_up = True
	with self._cleanup_lock:
	if self._cleaned_up:
	return
	self._cleaned_up = True

[AndreasKaratzas]

Python's GIL makes bool read/write atomic, and the underlying cleanup operations (close(linger=0), task.cancel(), setting attrs to None) are all individually idempotent. The _cleaned_up flag is an optimization to skip redundant work, not a correctness gate. Adding a Lock to a weakref finalizer target introduces complexity without practical benefit here.

[gemini-code-assist]

To support the thread-safe idempotency guard in __call__, a lock should be added to the BackgroundResources dataclass. This requires importing Lock from threading and field from dataclasses.

Suggested change

	_cleaned_up: bool = False
	_cleaned_up: bool = False
	_cleanup_lock: "Lock" = field(default_factory=Lock, init=False, repr=False)

[AndreasKaratzas]

cc @njhill @markmc

[AndreasKaratzas]

Adding the ready label for tests to start.

[njhill]

Thanks @AndreasKaratzas, I think this may already be fixed by #36270

[AndreasKaratzas]

Thanks @AndreasKaratzas, I think this may already be fixed by #36270

@njhill Oh let me test again without this then and let you know.

[AndreasKaratzas]

@njhill Indeed, closing.

[njhill]

Thanks @AndreasKaratzas 🙏