Storyyeller / Krakatau

Java decompiler, assembler, and disassembler
GNU General Public License v3.0
1.95k stars 220 forks source link

Fix race condition calling os.makedirs #40

Closed cernekee closed 9 years ago

cernekee commented 9 years ago

If multiple assemble.py instances are running concurrently (e.g. from make -j4), there is a potential race condition between the call to os.path.exists() and the call to os.makedirs(). Fix this by ignoring EEXIST errors from os.makedirs().

See also: http://stackoverflow.com/questions/16029871/how-to-run-os-mkdir-with-p-option-in-python

This was tested with Python 2.7.3.

Storyyeller commented 9 years ago

It seems like there are other race conditions too. I'm not sure how much of a concern that is.

For example, on Windows, it will keep track of previous filenames to avoid a collision due to case insensitivity, but since this is done on a per process basis, it won't help if you're running multiple instances in parallel.

It's also possible to have multiple assembly files that generate the same class, but I suppose the answer to that is "don't do this".

What do you think?

cernekee commented 9 years ago

I'm not sure how much of a concern that is.

FWIW, I did run into the makedirs race when assembling a large quantity of .j files back into .class files, but I didn't run into the others.

For example, on Windows, it will keep track of previous filenames to avoid a collision due to case insensitivity

This stems from the fact that a JAR can contain a.class and A.class, but a.class and A.class cannot coexist on a Windows filesystem. Correct?

So as long as the JAR is being unpacked by a single process/thread, and nobody is trying to unpack another JAR into the same directory, there is no need for synchronization.