BHoM / Python_Toolkit

GNU Lesser General Public License v3.0
3 stars 2 forks source link

Fixed deadlock on waiting for exit on some processes #138

Closed Tom-Kingstone closed 5 months ago

Tom-Kingstone commented 5 months ago

NOTE: Depends on

Issues addressed by this PR

Closes #137

Moved reading standard error and standard output to before waiting for exit on process. Turns out this was seen before, if you look at RunCommandStdOut, there is a comment that says to read stdout and stderr before waiting to exit to avoid deadlocks, but it must have been missed for the other methods. 🤦

Test files

Changelog

Additional comments

not very easy to test all of these, though InstallPackageLocal should be enough to prove that it works (as that is where the error was originally). do not merge unless @albinber has tested that it is fixed.

Tom-Kingstone commented 5 months ago

@BHoMBot check core @BHoMBot check compliance

bhombot-ci[bot] commented 5 months ago
@Tom-Kingstone to confirm, the following actions are now queued: - check `core` - check `code-compliance` - check `documentation-compliance` - check `project-compliance` - check `branch-compliance` - check `dataset-compliance` - check `copyright-compliance`
Tom-Kingstone commented 5 months ago

@BHoMBot check required

bhombot-ci[bot] commented 5 months ago
@Tom-Kingstone to confirm, the following actions are now queued: - check `code-compliance` - check `documentation-compliance` - check `project-compliance` - check `core` - check `null-handling` - check `serialisation` - check `versioning` - check `installer`
bhombot-ci[bot] commented 5 months ago
The check `code-compliance` has already been run previously and recorded as a successful check. This check has not been run again at this time.
bhombot-ci[bot] commented 5 months ago
The check `documentation-compliance` has already been run previously and recorded as a successful check. This check has not been run again at this time.
bhombot-ci[bot] commented 5 months ago
The check `core` has already been run previously and recorded as a successful check. This check has not been run again at this time.
FraserGreenroyd commented 5 months ago

@BHoMBot check ready-to-merge

bhombot-ci[bot] commented 5 months ago
@FraserGreenroyd to confirm, the following actions are now queued: - check `ready-to-merge`