Process-safe launching of minecraft instance to solve creating multiple minerl environments in parallel

[juliusfrost]

Adds a simple thread lock to prevent multiple processes from accessing the launch script. This prevents gradle build errors when trying to create multiple minerl environments in parallel. This passes asynchronous creation in #334 with the help of #351

[minerllabs]

Does this require waiting for one instance to lock then launching the next?

[juliusfrost]

@minerllabs Yes, one thread acquires the lock first, then other threads wait in a queue to acquire the lock after the first one exits. There could be a way to narrow the scope of the lock period, but so far as I see it, there isn't a good way as the logic is controlled in the Minecraft client. But in practice, I didn't see much of an issue with the locking, other than it taking a while to launch all of the environments.

[juliusfrost]

I found that this solution doesn't work with certain multiprocessing techniques to parallelize the environments, such as in RLlib. So this pull might not be the best to merge yet. I'll be looking into this.

[pep8speaks]

Hello @juliusfrost! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

• In the file minerl/env/core.py:

Line 51:80: E501 line too long (84 > 79 characters)

Comment last updated at 2020-07-14 20:43:18 UTC

[juliusfrost]

FileLock uses a file as communication for processes to proceed in the code. When a process enters the launch instance code, FileLock creates a file env.lock in the same directory and prevents other processes from entering unless the lock is removed.

[juliusfrost]

These changes require adding the filelock module to requirements.txt. I've replaced the thread lock to this instead to work with multiple processes. This is a necessary change to add multiprocessing support, but parallelization improvements can be made if direct modification with Malmo is involved.

[juliusfrost]

@MadcowD @minerllabs What should be done further to merge this?

[neevparikh]

Hey this would be nice to have @MadcowD @minerllabs. Can we approve this PR?

[danijar]

Any update on this?

[Miffyli]

This seems to have been fixed in the latest version. At least this test script runs fine (but for some reason runs slow on my machine, even with a single environment. Might be my mangled environments). Parallel instances do not work on Windows, tho.