facebook / wangle

Wangle is a framework providing a set of common client/server abstractions for building services in a consistent, modular, and composable way.
Apache License 2.0
3.04k stars 535 forks source link

Checking if file exists before reading can cause problem in case of race conditions. #154

Closed mohitanand001 closed 5 years ago

mohitanand001 commented 5 years ago

https://github.com/facebook/wangle/blob/cc46c238641eab405eb15ff14a693128f463a465/build/fbcode_builder/getdeps/fetcher.py#L390-L391

The above snippet of code can cause problem in case of race condition, where file might be present at the time of if os.path.exists(path) but might not exist when we try to read the file and result in exception. The python EAFP(Easy to Ask for Forgiveness than Permission) philosophy suggests that we use a try catch block to avoid this problem. Suggested changes. You can read this stackoverflow answer to understand the problem more Can this issue be assigned to me? The above snippet can be replaced by the following.


try:
    f = open(config_file)
except IOError as e:
    print("File can't be accessed")
else:
      with f:
             remaining code
              ...
              ...

@wez 
@yfeldblum 
yfeldblum commented 5 years ago

This build system should not be used concurrently with other programs which can modify the build system's filesystem state.

But if the user insists on using it that way and the build system fails with a raised exception, the user may simply retry.