aws-deadline / deadline-cloud

Multi-purpose library and command line tool that implements functionality to support applications using AWS Deadline Cloud.
Apache License 2.0
25 stars 28 forks source link

fix: use devnull for stderr pipe to dcm process handle #421

Closed spkamboj closed 2 months ago

spkamboj commented 2 months ago

What was the problem/requirement? (What/Why)

When using deadline auth login and deadline gui for login using Deadline Cloud monitor, there is race condition that Deadline Cloud monitor process handle closes early while the application is starting. This closes the stderr pipe opened with Deadline Cloud monitor process, and any messages sent to stderr results in panic, crashing the Deadline Cloud monitor.

What was the solution? (How)

The error messages in stderr are coming from crates used in DCM, which login flow does not need to know. DCM itself sends login failure message over stdout. This change is pointing process stderr pipe to devnull to prevent panic.

What is the impact of this change?

Fix the race condition which crashes the Deadline Cloud monitor application during login.

How was this change tested?

hatch build
hatch run fmt
hatch run lint
hatch run test
...
Required test coverage of 80.0% reached. Total coverage: 83.28%
=============================================================================================== slowest 5 durations ===============================================================================================
6.16s call     test/unit/deadline_job_attachments/test_upload.py::TestUpload::test_asset_management_no_outputs_large_number_of_inputs_already_uploaded[2023-03-03-200]
5.06s call     test/unit/deadline_job_attachments/test_progress_tracker.py::TestProgressTracker::test_increment_race_condition
4.96s call     test/unit/deadline_job_attachments/test_upload.py::TestUpload::test_asset_management_many_inputs[2023-03-03-200]
3.98s call     test/unit/deadline_job_attachments/test_upload.py::TestUpload::test_asset_management_no_outputs_large_number_of_inputs_already_uploaded[2023-03-03-100]
3.41s call     test/unit/deadline_job_attachments/test_upload.py::TestUpload::test_asset_management_many_inputs[2023-03-03-100]
============================================================================= 1326 passed, 46 skipped, 1 xfailed, 1 warning in 28.98s =============================================================================

Was this change documented?

Not applicable, no APIs or customer behaviors are changed.

Is this a breaking change?

No


By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

spkamboj commented 2 months ago

I assume you have tested this with DCM?

Yes, I tested it with DCM on all three platforms.

sonarcloud[bot] commented 2 months ago

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud