Race condition when multiple users use isolate about isolate HOT 11 CLOSED

If we use the first solution, I also need to know how to signal that the fail was because a box already existed. A different exit code? Or just exit(1) and require the user to parse the output message themself to know that they need to retry?

from isolate.

from isolate.

I agree with @gollux, isolate should not care about how you use it's boxes. @seirl you said about "proper" way of handling race condition

The "proper" way to do that is to list the directories in /var/lib/isolate/ and to --init with an available ID. But between the time you check that an ID is available and the time you call --init, another program might have done a --init for the same ID and you get the same cgroup and sandbox for both.

I don't think this is "proper" way, because problem you have lies inside your solution of handling race conditions. You should change your solution and not isolate. 😄

As an example please take a look at Judge0 API. REST API that directly uses isolate to run untrusted programs. There, this race problem is solved by building Submission model which is stored in the database. It's unique ID inside database is used as box ID.

from isolate.

All of what you are describing is already what we are doing.

The problem is when someone decides to run two instances of our program, for instance a server that handles the requests (like what judge0 is doing) is already running, and then someone else decides to run the unit tests on the same machine.

I'm pretty sure running Judge0 alongside with an isolate testsuite will cause the same problem, they just ignore the problem completely from what I've seen.

from isolate.

I now better understand what your problem is, but I still believe that isolate should not be the one fixing it. Isolate provides simple interface for running untrusted code on your server/machine. If you have concurrency problems when creating isolate boxes, then you should create another layer of abstraction above isolate which solves your problem. So you could create a program which has the same interface as isolate (or at least similar) which solves your concurrency problem, and this program should use isolate "under the hood".

"Quick" fix for you problem could be one of the following:

• Run tests on another server, not production server
• On your server compile two isolates. Production isolate which uses default box_root, and the other which uses some other box_root. See default.cf. Then use production isolate for production server, and this other isolate for test environment.
• Wrap your application inside Docker

Also worth mentioning. Yes, Judge0 API has the same problem if you install it on your server in the "old school way". But main power of it is its mobility from machine to machine. And that is achieved with Docker. So every instance of Judge0 is run inside Docker container which has its own isolate.

from isolate.

While what you're saying makes sense, I think this is the only issue that prevents different programs from using isolate at the same time, and it's pretty minor so it could be easily changed.

Adding a --require-empty option is trivial and has some real value even for single programs, as it might also help find problems where you reuse a box ID that has not been properly --cleanup'd. For instance, if you forgot to --cleanup the lingering box IDs you're using when you start your frontend after a server hard reset, it might be nice to have that extra check that tells you "hey, you're trying to --init something that already exists, are you sure you want to do that?" and it would pretty much solve the RC problem for testsuites etc.

Are you really opposed to making that small change?

from isolate.

from isolate.

from isolate.

Added pull request #27 as a followup to @gollux 's first suggestion.

from isolate.

I think having --init failing when the directory isn't empty is the
best way to go.

+1

Silently "fixing" the error when assumptions seems to have been broken feels scary. Much better to fail early and controlled.

from isolate.

I also realized that another advantage of that PR is that it will ensure people check the exit code of their --cleanup call instead of assuming that it always work :-)
For instance, if the cgroup has not been deleted for some reason you really don't want to run something in the same box again, or else you might reuse that box.

It also forces you to cleanup everything when you start your frontend instead of assuming the directory is empty.

from isolate.