Multi-threading: checking that the student use multiple threads

[AlexandreDubray]

In module 6, we introduce the students to multi-threading programming.
In the exercises, it is diffcult to actually test if they use or not multiple threads. And even if they use multiple threads, are they doing what they should be doing?

It seems that we can access the runtime stack trace of the threads using this method.

We should investigate how this can be translated in the exercise.

[AlexandreDubray]

It seems that it is doable in a reasonable way. I did a POC here

LEPL1402/ParallelCounting/src/InginiousTests.java

Line 79 in 35121f8

public void testNumberThreads() {

The idea is the following:

1. We create a thread that will update the number of threads launched by the student.
2. Then we set it as a deamon and start it. According to the Java 8 doc the JVM exits when only deamon threads are still running. So we do not have to worry about closing it (let's keep the test code simple?)
3. The thread need to keep track of the threads it saw in a Set<Threads>. Because we can not predict how the threads will be executed (particulary in a test environment like INGInious), it is not sufficient to record the peak thread number.
4. The thread will continuously use Thread.getAllStackTraces() method to get the stack trace of the threads and then iterate over the keys of this map to get the threads that are running. We can then count the threads that are not yet seen.
5. Before launching the code of the students, we need to get the threads already launched by the JVM
6. This code might be problematic if the GC is launched. ATM we just keep the test case small so the GC is not called.

In java code, it looks like that

@Test
public void testNumberThreads() {
    HashSet<Thread> threadsSeen = new HashSet<>();
    int [] threadLaunchedByStudentCode = new int[]{0};
    Thread t = new Thread(new Runnable(){
      public void run() {
          while (true) {
              for (Thread th : Thread.getAllStackTraces().keySet()) {
                  if (!threadsSeen.contains(th)) {
                      threadLaunchedByStudentCode[0] += 1;
                      threadsSeen.add(th);
                  }
              }
          }
      }
    });
    t.setDaemon(true);
    t.start();
    
    for (Thread th : Thread.getAllStackTraces().keySet())
      threadsSeen.add(th);
    int nThreads = 4;
    StudentCode.run(..., nThreads);
    Assert.assertEquals(nThreads, threadLaunchedByStudentCode[0]);
}

/!\ this method to test is not stable with respect to thread executions (some threads might be forgotten) /!
A quick fix is to test that the students launched at least one thread and assume that if they launch 1 other thread, they launch nThreads.

A more durable way of doing things might be to add bytecode the the Thread.start method using this API

[AlexandreDubray]

Related to this type of questions (I need to log this so we do not forget it).

When doing the exam we ran into a problem with an exercise on threads: the Inginious task timed out (the container) with time out for the tasks set to 1 second. This should obviously never happen.

Below is the minimal code to reproduce the error. The code of the student to be evaluated:

import java.util.concurrent.Executors;
import java.util.concurrent.ExecutorService;

public class Code {
    public static void main(String [] args) {
        ExecutorService service = Executors.newFixedThreadPool(10);
        service.submit(() -> {});
        //service.shutdown();
    }

}

Notice how the ExecutorService is not shutdown. When the main function exits, the threads in the ExecutorService are still up (see here). As explained here the threads are create as non-daemon thread. While they are active, the JVM will not exit.

Thus when the ExecutorService is not explicitely shutdown, the java command will not terminate and cause the container to time out.

A quick fix would be to do a System.exit(0) after the test are finished. However this does not force the students to shutdown the ExecutorService. Moreover, even if they do shutdown it, if the test timeout the service.shutdown() line is never called and thus the same problem arises.

Another idea, more scalable, is to shutdown ourselve the service using reflection. We just need to add a check to do that at the end of every test and update a flag is the service is not shutdown. Then we can add a test to test the value of the flag.