Interactive problems checkers are not very robust

[meisterT]

Description of the problem / feature request

Support interactive problems properly, preferably by supporting the problemtools format, see https://github.com/Kattis/problemtools/tree/develop/examples/guess for an example similar to our own boolfind.

Currently it's easy to block a judgedaemon or produce "incorrect" verdicts because of how we separate running from validation.
We should consider adding a proper wrapper to both runner / validator scripts during problem import to make setting up these problems as easy as possible.

Steps to reproduce

Example of a incorrect/malicious submission:

int main() {
	while (true) {
		cout << "READ 0" << endl;
	}
	return 0;
}

Expected behaviour

Wrong answer.

Actual behaviour

Judgedaemon is hanging.

Any other information that you want to share?

There are other problematic submissions as well, I'll add examples in a later post when I got them.

[meisterT]

I looked into this yesterday evening a bit. We do kill the submission but it ends up in defunct/zombie state. This is because we don't call wait/waitpid. After adding more debug statements, I found that we hang in either slice/read (depending on what type is used).

So my current idea is the following:
While processing output from the program, we receive SIGALRM in the parent process and then process it. We do a SA_RESTART, so we restart the system call after handling the signal. This then seems to block indefinitely.

The pstree looks like this:

php(13445)───sh(13566)───testcase_run.sh(13567)───runpipe(13628)─┬─runjury(13629)
                                                                                        └─sudo(13630,root)───runguard(13631)

@eldering I did look for the pipe picture of how we set up judging for interactive problems, but haven't found it. Do you know where it lives?

[eldering]

new-io-redirection.pdf

I think I never committed that image, but just sent it to domjudge-devel. See attached.

Oh, and the runpipe helper that's executed by the run script is missing here.

[meisterT]

Thanks for the image, that looks roughly like what I had in mind.
Since adding an image doesn't fix the bug, I'll reopen it ;-)

[meisterT]

I investigated a bit more.

In the runpipe we wait until both children have exited (note that not the team submission is one of them but runguard). After that we would close the pipes / file descriptors. But the children didn't exit so far.

In runguard, we killed the team submission which is in zombie state because we didn't wait for it yet. We do hang on a blocking read, so we don't reach the waitpid which would allow us to terminate ourselves.

My current (not well thought out) guess is that we should merge runpipe into runguard.

[meisterT]

@eldering what is this code supposed to do?https://github.com/DOMjudge/domjudge/blob/master/judge/runpipe.c#L252-L253
I don't get why we do a blocking wait right after the non-blocking wait, but perhaps I'm missing something.

Anyway, Jaap mentioned in IRC that merging runpipe into runguard is a lot of change which we probably don't want if we can avoid it.
So I toss another idea around:

• add option --notifyontle <pid> to runguard
• special case runpipe to make it aware which command is which and add own pid to runguard's arguments
• send SIGUSR1 from runguard to runpipe when we run into SIGALRM
• do the "right thing" in runpipe on SIGUSR1 receival, i.e. probably close all fds, so that the normal flow in runguard that keeps reading from the pipes can finish

@eldering, since you are clearly the best person to fix this, I'll assign the bug to you :-)

[meisterT]

Thanks GitHub. Before we can close is not the same as actually close it now ;-)

[meisterT]

Update: this is now mostly solved and interactive problems work in the problemtools format. There is still room for improvement, but I don't know if any functional changes that are still to do.

[thijskh]

So then the issue can be closed?

[meisterT]

I'll do some more testing (probably to evening) and close it afterwards.

[meisterT]

I've done more testing and integrated the problemtools example in our travis test. I also sent Kattis/problemtools#112