fix(perforce): Isolate P4 trust/ticket files per client to prevent lock contention#109652
Merged
fix(perforce): Isolate P4 trust/ticket files per client to prevent lock contention#109652
Conversation
…ck contention
Perforce SSL connections were failing with "Unrecoverable lock error
'/home/sentry/.p4trust.lck'" because all tenants shared the same global
trust and ticket files (~/.p4trust, ~/.p4tickets, ~/.p4enviro).
Changes:
- Create a per-PerforceClient temp directory for .p4trust and .p4tickets,
isolating each tenant's SSL trust and session ticket state.
- Use p4.set_env("P4TRUST", ...) instead of the non-existent p4.trust_file
property (which raises "No string attribute with name trust_file").
- Redirect P4ENVIRO to /dev/null at module level to prevent set_env() from
writing to the shared ~/.p4enviro file on disk. The per-instance in-memory
value (which P4 operations actually use) is still set even when the disk
write fails.
- Bind temp directory lifetime to the PerforceClient instance so trust/ticket
state is cached across multiple _connect() calls within the same client.
- Add tests verifying per-instance isolation, P4ENVIRO redirect behavior,
in-memory value survival on disk write failure, and concurrent safety.
github-actions
bot
added
the
Scope: Backend
Contributor
Backend Test FailuresFailures on
|
…rceWarning Python 3.13 raises ResourceWarning when TemporaryDirectory is cleaned up implicitly by the garbage collector rather than via explicit context manager exit. Since PerforceClient stores the temp dir as an instance attribute (not in a with block), GC cleanup triggers this warning and fails CI. Switch to mkdtemp + atexit.register(shutil.rmtree) for explicit cleanup without relying on GC finalizers. Co-Authored-By: Claude <noreply@anthropic.com>
Contributor
Backend Test FailuresFailures on
|
…emp+atexit Replace mkdtemp+atexit with TemporaryDirectory inside _connect() to properly scope temp directory lifetime. The atexit approach only ran cleanup at process shutdown, causing temp dirs to accumulate in long- running workers. The context manager cleans up immediately when the connection context exits. Also fix three test failures: - Remove broken test_connect_does_not_set_trust_file_property (Mock attribute assertion was incorrect, and the behavior is already verified by test_connect_sets_isolated_trust_and_ticket_paths) - Add try/except around set_env calls in tests that use real P4 instances (set_env raises when P4ENVIRO is /dev/null) - Remove tests that relied on __init__ tmpdir (no longer exists) Co-Authored-By: Claude <noreply@anthropic.com>
cmanallen
approved these changes
Mar 2, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Perforce SSL connections were failing with "Unrecoverable lock error '/home/sentry/.p4trust.lck'" because all tenants shared the same global trust and ticket files (~/.p4trust, ~/.p4tickets, ~/.p4enviro).
Changes: