0

I read a lot about Java Threads but I am not sure about the optimal solution.

I create a worker thread to access a php-Script (and the php-Script accesses a mysql-database). If somehow the server with the php-Script or the mysql-database is busy the Thread freazes in a read or send operation. Therefore the concept of setting an interupt and have the Thread stop itself does not work.

Now I created a second worker thread with a ProgressMonitor. When the user clicks the cancel button of the ProgressMonitor the frozen first Thread is canceled. In case the first Thread works okay, it cancels the second Thread. Therefore the two Threads can cancel each other.

But is this the optimal solution? Is there a better and safer way to do this?

    class ArbeiterErstelleTabellenmodell extends SwingWorker<TabellenmodellMitarbeiter, Object>
{
    ProgressMonitor anzeige;
    ErstelleTabellenmodellMitAnzeige fadenAnzeige;

    ArbeiterErstelleTabellenmodell(ProgressMonitor anzeige, ErstelleTabellenmodellMitAnzeige fadenAnzeige)
    {
        this.anzeige = anzeige;
        this.fadenAnzeige = fadenAnzeige;
    }

    @Override 
    public TabellenmodellMitarbeiter doInBackground()
    {       
        this.anzeige.setProgress(0);
        this.anzeige.setNote("1.) Datenabfrage aufrufen ...");

        TabellenmodellMitarbeiter tm = new TabellenmodellMitarbeiter();     
        String daten = null;

        try 
        {   
            URL url = new URL("http://www.greif-integra.de/daten/php/mitarbeiter/select_mitarbeiter_tabelle.php");                  
            PhpPostConnect con = new PhpPostConnect(url);

            this.anzeige.setProgress(30);
            this.anzeige.setNote("2.) Daten lesen ...");

            try
            {
                daten = con.read();

                this.anzeige.setProgress(60);
                this.anzeige.setNote("3.) Daten aufbereiten ...");

                // here the received data is being processed
            }
            catch (IOException e)
            {
                meldungTabelle.setText("FEHLER Die Tabelle kann nicht angezeigt werden. IOException");
            }
        }
        catch (MalformedURLException e)
        {
            meldungTabelle.setText("FEHLER Die Tabelle kann nicht angezeigt werden. MalformedURLException");
        }
        catch (Exception e)
        {
            meldungTabelle.setText("FEHLER Die Tabelle kann nicht angezeigt werden. Exception");
        }

        this.anzeige.setProgress(90);
        this.anzeige.setNote("4.) Die Tabelle erzeugen ...");

        return tm;
    }

    @Override protected void done()
    {       
            // some work with the data is done here

        this.fadenAnzeige.cancel(true);
        this.anzeige.close();
    }
}

In my Java program I start and execute an object of this second class.

    class ErstelleTabellenmodellMitAnzeige extends SwingWorker<Object, Object>
{       
    @Override
    protected Object doInBackground()
    {
        ProgressMonitor anzeige = new ProgressMonitor(KarteMitarbeiter.this,
                                                      "Fortschrittsanzeige",
                                                      "",
                                                      0,
                                                      100);

        ArbeiterErstelleTabellenmodell fadenTabellenmodell = new ArbeiterErstelleTabellenmodell(anzeige, this);
        fadenTabellenmodell.execute();

        while(true)
        {
            try
            {
                Thread.sleep(500);
            }
            catch (InterruptedException e)
            {}

            if(anzeige.isCanceled())
            {
                fadenTabellenmodell.cancel(true);
                break;
            }
        }

        anzeige.close();
        return null;
    }
}

Maybe there is no optimal solution. I just want to make sure because I want to use the software every day. Thank you in advance. I appreciate your ideas.

  • 1
    Just one thing... Many people don't understand German ;) I have no doubt you have class names which are meaningful in German, but if these names were in English you'd have more people able to help without having to browse through the whole code to understand which class does what. – fge Mar 02 '14 at 11:29
  • Sorry I understand your point. It just would be too much work to change all the names to English names :-) and I probably would screw up the code in the process ;-). –  Mar 02 '14 at 11:47
  • I don't see any improvement from having two worker threads. Your fundamental issue is the usage of non-interruptible blocking I/O operations. – Marko Topolnik Mar 02 '14 at 12:03
  • Yes, how to deal with that? Does that mean even trying to cancel the blocked thread from the outside will not work? –  Mar 02 '14 at 12:38
  • #Marko Topolnik Thank you for your comment. Maybe it would be good to alter my connection class by "implements InterruptibleChannel". Still I discovered there is a different bug in my code: it was a bad idea to start one Thread from another because then they do not work independent of each other. –  Mar 02 '14 at 13:33
  • @user3327856: *It just would be too much work to ...* - sorry, but no. You want people to answer your question and provide help. Therefore you don't want to make it hard to them to do that, just because you are too lazy to use the refactoring/rename function. You will get the amount and quality of answers you deserve. – JensG Mar 02 '14 at 16:26

3 Answers3

0

Therefore the two Threads can cancel each other. But is this the optimal solution? Is there a better and safer way to do this?

The best solution is to set a volatile flag which is being checked regularly to see if the thread should stop running. If you are blocking on a Stream, you can close the stream and cause it to trigger an IOException (like AsynchronousCloseException for Channels)


If you really have no choice, you can use thread.stop(); This will cause the thread to throw a ThreadDeath Error which should trigger it to unwind the stack (and any locks) and cause your thread to die. This should only be used as a last resource and it could leave data in an inconsistent state. i.e. the error could be thrown on any line.

Note: If you catch Throwable, or Error or ThreadDeath, this will catch this error, just like any other and the thread won't die.

Peter Lawrey
  • 525,659
  • 79
  • 751
  • 1,130
0

You can get rid of one thread as long you as you are notified that the user clicked "Cancel". I made a working example using code from the answers here and here.
You will need to download the SwingUtils class to make the example work.

import java.awt.GridLayout;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import java.util.concurrent.atomic.AtomicReference;

import javax.accessibility.AccessibleContext;
import javax.swing.JButton;
import javax.swing.JDialog;
import javax.swing.JFrame;
import javax.swing.ProgressMonitor;
import javax.swing.SwingUtilities;

public class Q22126862 {

public static void main(String[] args) {

    SwingUtilities.invokeLater(new Runnable() {

        @Override
        public void run() {
            new SwingWorkerExample();
        }
    });
}

static class SwingWorkerExample extends JFrame implements ActionListener {

    private static final long serialVersionUID = 1L;
    private final JButton startButton;
    private MySwingWorker swingWorker;

    public SwingWorkerExample() {
        super("SwingWorkerExample");
        setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        getContentPane().setLayout(new GridLayout(2, 2));
        startButton = makeButton("Start");
        //Display the window.
        pack();
        setVisible(true);
    }       

    private JButton makeButton(String caption) {

        JButton b = new JButton(caption);
        b.setActionCommand(caption);
        b.addActionListener(this);
        getContentPane().add(b);
        return b;
    }

    @Override
    public void actionPerformed(ActionEvent e) {

        if ("Start".equals(e.getActionCommand())) {
            startButton.setEnabled(false);
            // Note that it creates a new instance of the SwingWorker-derived class. Never reuse an old one.
            ProgressMonitor progressMonitor = new ProgressMonitor(this, "Sleep progress", "sleeping", 0, 99);
            (swingWorker = new MySwingWorker(this, progressMonitor, 3000, 10)).execute(); // new instance
        } else if ("TaskDone".equals(e.getActionCommand())) {
            startButton.setEnabled(true);
            System.out.println("SwingWorker task finished OK: " + swingWorker.getResult());
        } else {
            System.out.println("Unknown action: " + e);
        }
    }
}

static class MySwingWorker extends javax.swing.SwingWorker<Boolean, Void> implements ActionListener {

    private final ActionListener taskListener;
    private final long sleepMs; 
    private final int sleepSteps;
    private final ProgressMonitor progressMonitor;
    private final AtomicReference<Thread> currentThread = new AtomicReference<Thread>();
    private volatile boolean done;
    private JButton cancelButton;
    private boolean result;

    public MySwingWorker(ActionListener taskListener, ProgressMonitor progressMonitor, long sleepMs, int sleepSteps) {
        super();
        this.taskListener = taskListener;
        this.sleepMs = sleepMs;
        this.sleepSteps = sleepSteps;
        this.progressMonitor = progressMonitor;
    }

    @Override
    protected Boolean doInBackground() {

        currentThread.set(Thread.currentThread());
        long sleepTimeMs = sleepMs / sleepSteps; 
        try {

            // Initialize the progress monitor so that it has a backing JDialog
            progressMonitor.setMillisToDecideToPopup(0);
            progressMonitor.setProgress(0);
            AccessibleContext ac = progressMonitor.getAccessibleContext();
            JDialog dialog = (JDialog)ac.getAccessibleParent();
            java.util.List<JButton> components = darrylbu.util.SwingUtils.getDescendantsOfType(JButton.class, dialog, true);
            cancelButton = components.get(0);
            cancelButton.addActionListener(this);

            for (int i = 0; i < sleepSteps; i++) {
                Thread.sleep(sleepTimeMs);
                int progress = (int)((i / ((float)sleepSteps)) * 100.0);
                progressMonitor.setProgress(progress);
                System.out.println("Sleep progress: " + progress);
            }
            result = true;

        } catch (Exception e) {
            //e.printStackTrace();
            System.out.println(e.toString());
        } finally {
            done = true;
            currentThread.set(null);
            System.out.println("Background task done");
        }
        return result;
    }

    public Boolean getResult() {
        return result;
    }

    @Override
    protected void done() {

        System.out.println("Task done");
        progressMonitor.close();
        System.out.println("Monitor closed");
        ActionEvent e = new ActionEvent(this, 0, "TaskDone");
        taskListener.actionPerformed(e);
    }

    protected void cancel() {

        if (done) {
            return;
        }
        Thread t = currentThread.get();
        if (t != null) {
            t.interrupt();
        }
        // In case if I/O-tasks, close the source that is read from (e.g. socket).
        // Interrupting a blocked reading thread has no effect in this case. 
    }

    @Override
    public void actionPerformed(ActionEvent e) {

        System.out.println("Action: " + e);
        if ("Cancel".equals(e.getActionCommand())) {
            cancel();
        }
    }
}

}
Community
  • 1
  • 1
vanOekel
  • 6,358
  • 1
  • 21
  • 56
0

Marko Topolnik got me on the right track:

I don't see any improvement from having two worker threads. Your fundamental issue is the usage of non-interruptible blocking I/O operations. – Marko Topolnik

The blocking I/O had to be changed.

I am using URLConnection and now I discovered that it is possible to do

    .setConnectTimeout(1500);
    .setReadTimeout(1800);

Now I do not need to stop the WorkerThread because the IO Operation will throw an exception on the WorkerThread when it is stuck.

Anyway before I found this easy solution I did the trick by using a get() with a timeout-parameter to retrieve the result from the WorkerThread. This throws a TimeoutException which can be used to do cancel() on the WorkerThread. The WorkerThread has to check on if(canceled())return; to stop itself.