autopkg / MLBZ521-recipes

AutoPkg Recipes
MIT License
14 stars 22 forks source link

Race condition in Solstice recipe #31

Closed dcoobs closed 3 years ago

dcoobs commented 3 years ago

I encountered an issue in the Solstice recipe where the Info.plist hadn't been generated yet before the process building the app was killed, resulting in failures. Incrementing the sleep from 5 to 6 seconds resolved the issue. I'm wondering if there should be a check for the existence of the Info.plist file before the script continues to somewhat mitigate this?

Perhaps something like the following so that the processor sleeps while Info.plist doesn't exist (or timeout after 8 seconds):

while not os.path.exists(file_path):
    time.sleep(1)
    time_counter += 1
    if time_counter > 8:break
build.kill()

https://github.com/autopkg/MLBZ521-recipes/blob/0f5c41fbb2c9ca10f4eb998b6fa1a57fde74483d/Solstice/SolsticeProcessor.py#L87-L89

MLBZ521 commented 3 years ago

@dcoobs Yeah, I'm not surprised by a race condition with this process at all. And depending on the system it's being run on and a bunch of factors that could play into it, it's very possible.

Your proposed solution is wise. 🤦 Can't believe I didn't think about adding something that simple.

Would you like to submit a PR for this? Otherwise, I'd be happy to implement it.

dcoobs commented 3 years ago

Thanks for getting this done! Looks like there's a missing closing parenthesis. Should be fixed in https://github.com/autopkg/MLBZ521-recipes/pull/32

https://github.com/autopkg/MLBZ521-recipes/blob/c1f2922f08c119f8902680bd98c9ca7849184be3/Solstice/SolsticeProcessor.py#L94

MLBZ521 commented 3 years ago

Last thing I did before calling it a night and didn't review well enough...

Merged