Deal with some race conditions in mkpath

[samkohn]

From my commit message:
Although mkpath checks to see which parent directories need to be
created, there is a race condition from the time of that check and the
actual creation of directories. In the event another process creates one
of those directories during that time, this commit will suppress the
mkdir error due to the file already existing. Addresses issue #13094.

Additionally:
As mentioned on the issue thread, it's impossible to prevent all race conditions in this situation, but I think this is a reasonable place to stop. If some other process creates one of the intended directories, then great, no error should occur. On the other hand, if the other process deletes one of the directories, then an error is a good thing in my opinion.

[samkohn]

The Travis build is failing when it tries to (I think) download and install the dependencies (https://travis-ci.org/JuliaLang/julia/jobs/87760084#L3123 and next 50 lines). I don't think this is an issue from my commit (although of course, one can never be sure). Is there a way to re-run Travis?

[tkelman]

Yep, good reading of the log, I restarted it and looks fine.

I'm not sure whether that type assertion catch err::SystemError does what you're going for here, this is a tricky race condition to artificially induce or test for. You might have to check isa(err, SystemError) within the catch block.

(p.s. good to see another contributor from Berkeley - there's a Bay Area Julia Users' meetup on Thursday, you should come)

[samkohn]

Fair enough. I'm still learning some of the syntax. Is there a way (other than using isa) to filter caught exceptions based on type? I'm thinking of the python try: . . . except SomeException as e: . . . syntax. If that's what catch err::SystemError does, then that's what I want. If it tries to convert whatever is thrown into a SystemError, then that's definitely not what I want.

I would have just used isa because that clearly works—I just have a small python-induced bias against explicitly checking type, plus the try-catch system seems like it would handle exception types more efficiently than explicit checks.

And thanks for the invite to the meetup. I'm busy tomorrow but I'll check the calendar for future meetings.

[tkelman]

I don't think we have a dispatch-based catch implemented, it would be cool if it worked that way though.

julia> try
           throw(ErrorException("an error"))
       catch err::ArgumentError
           println("let's see")
       end
ERROR: UndefVarError: err not defined

Might be a bug that we get an UndefVarError here, I would think it might instead fail a type assertion as in

julia> a = 5.0::Int
ERROR: TypeError: typeassert: expected Int64, got Float64

[samkohn]

Hm, I guess I'll search through the issues to see if that already exists or if people have already decided it's not a good idea. And I'll fix my code and push again.

[samkohn]

Also the UndefVarError seems to be because the catch err::ArgumentError is parsed not as a new variable declaration but as a type conversion of a hopefully existing variable err to type ArgumentError. (Try catching the same type of exception that you throw using this syntax…the same error happens.) I agree that that doesn't seem to be the clearest sort of error message.

[samkohn]

Apparently the syntax I used was proposed and rejected in favor if if statements and rethrow back in 2012: #1075.

[samkohn]

Time to test?

[vtjnash]

isdir is a fairly expensive syscall while isa is essentially a free pointer comparison. (although this is the failure case just after trying an even more expensive mkdir syscall, so the relative performance probably isn't too important)

[samkohn]

On second look, it seems that the only error mkdir ever throws is a SystemError, so the isa call is not necessary.

[tkelman]

only error thrown directly, though i wonder whether any nested calls at deeper levels would throw anything else

[samkohn]

I couldn't figure out an instance where that would happen. Of course that's no guarantee. That being said, which of the following is our desired behavior:

• Suppress the "file exists" system error and continue making new directories
• or, suppress any error as long as directories continue to be created

[tkelman]

I'd prefer the former. Suppressing all errors without checking the reason for them too carefully can be dangerous and conceal bugs in deeper layers.

[samkohn]

Fair enough. I pushed a new commit taking that into account.

[samkohn]

I updated the catch block to reflect @vtjnash's comment

[tkelman]

Can you squash the commits? I think this is good to merge after that, any objections?

[samkohn]

Travis and AppVeyor are happy with the final squashed commit.