3

I'm writing a Java program involving a multithreaded worker pool of Processes. Each instance of the class Process needs to be able to spawn a single additional thread to do some work. But the thread should by spawned by the instance itself and nobody else. Unfortunately Runnable.run is 'public' so I cannot really enforce this without doing some trick.

Here is the trick I plan to use:

  1. Implement Runnable in Process
  2. Write this into the implementation:

Code:

 class Process implements Runnable {

    private boolean threadkey = false;

    public void run() {

        synchronized(threadkey) {

            // is someone wrongly calling 'run'?
            if(!threadkey)
                return;

            /* switch off threadkey to make sure
            it cannot be called meaningfully again */
            threadkey = false;
        }
        /* continue processing
         *
         */
        return;
    }

Of course, now all I will need to do when I want to run legitimately is to switch on 'threadkey' (which is private) before making the call.

Elegant? Or not? Or is there a better way? Or should I just not bother with enforcing this and write a neat little comment explaining NOT to call 'run'?

Do people even call 'run' from within the class that needs to do the 'running'?

ArjunShankar
  • 23,020
  • 5
  • 61
  • 83

4 Answers4

12

While you are correct that the run method of a Runnable is public, one thing you can do to prevent other people from calling it is to make your entire Runnable implementation a package-private class or a private inner class. This way, while it may have a public run method, code other than your custom class cannot instantiate the Runnable object. If you also have your class not hand back references to it, then clients won't have access to the Runnable, and so they can't invoke run. That is, don't have your Process class implement Runnable; instead, make another class that actually implements Runnable, then make Process the only class that can access it.

This approach is more elegant than what you have proposed because it prevents other code from calling run at compile-time rather than at runtime. In particular, if any code tries to call your run method described above, it will compile just fine, and will fail at runtime. The problem is that the code will compile but can never work correctly. Making the Runnable inaccessible means that if someone does try to run it, they'll get a compile-time error and will have to rethink their design. In other words, the compiler will detect the error before it even happens.

In general, if you ever find yourself wanting to prevent random code from calling methods of a class that have to be public because they're declared in an interface, consider changing the access specifier for the entire class so that clients can't reference it.

templatetypedef
  • 362,284
  • 104
  • 897
  • 1,065
7

You can use an anonymous inner class of Runnable like this:

private void spawnThread() {
    Thread t = new Thread( new Runnable(){
          public void run(){
             // do something
          }
    }
    t.start();
}

And spawnThread() can only be called from within the class as it is private.

Alb
  • 3,601
  • 6
  • 33
  • 38
1

You can have you run() method private, and use an inner class (implementing Runnable) to access it:

public class Process {

  private Thread extraThread = null;

  private void run() { 
      // Do you thing
  }

  public synchronized void spawnExtraThread() {
    if(extraThread != null)  
      return; // Do nothing an extra thread was already spwaned
    extraThread = new Thread() {
      public void run() { Process.this.run(); }
    };
    extraThread.start();
  }
}

The main point is that class Process() no longer implements Runnable and therefore can no longer be turned into a Thread. Of course you extend this design to support any limit on the number of threads (just change the extraThread field into an array of Threads).

Itay Maman
  • 30,277
  • 10
  • 88
  • 118
0

Just wanted to add my two cents, an elegant solution using Executors and the double colon operator (::) that hasn't been mentioned yet:

import java.util.concurrent.Executors;

public class PrintFoo {

    public void run() {
        Executors.newSingleThreadExecutor().execute(this::doRun);
    }

    private void doRun() {
        for (int i = 20; i --> 0;) System.out.println("Foo");
    }
}

And then the class that calls it:

public class Main {
    public static void main(String[] args) throws InterruptedException {
        new PrintFoo().run();
        System.out.println("Going to sleep...");
        Thread.sleep(10000L);
    }
}

Output:

Going to sleep...
Foo
Foo
Foo
Foo
Foo
Foo
Foo
Foo
Foo
Foo
Foo
Foo
Foo
Foo
Foo
Foo
Foo
Foo
Foo
Foo
Kein
  • 348
  • 1
  • 3
  • 15