Removing a println call in java is causing a logical error

Question

I've been working on a program for a few weeks now and someone gave me a suggestion on how to improve it's performance which I am attempting to implement. It is so far working however, I have run into the issue that I cannot comment out one of the println calls I had previously added to help debug and still get the desired output. the code in question is

if (distTable != null)
{
    //System.out.println("test");
    float [] distLine;
    for (int i = 0; i < s.emitters.length; i++)
    {
        int tx = s.emitters[i].getX();
        int ty = s.emitters[i].getY();
        for (int x = 0; x < matrix.length; x++)
        {
            distLine = distTable[Math.abs(x-tx)];
            for (int y = 0; y < matrix[x].length; y++)
            {
                    matrix[x][y] += s.emitters[i].amplitudeAtDist(distLine[Math.abs((y * targetThreads)+threadNumber - ty)]);
            }
        }
        System.out.println("test " + i + " " + threadNumber);
    }
    callback.alertComplete(this);
    return;
}

the above code works as expected however, the code below does not and instead the program produces no output at all. The only difference is that the code below has one println call commented out.

if (distTable != null)
{
    //System.out.println("test");
    float [] distLine;
    for (int i = 0; i < s.emitters.length; i++)
    {
        int tx = s.emitters[i].getX();
        int ty = s.emitters[i].getY();
        for (int x = 0; x < matrix.length; x++)
        {
            distLine = distTable[Math.abs(x-tx)];
            for (int y = 0; y < matrix[x].length; y++)
            {
                    matrix[x][y] += s.emitters[i].amplitudeAtDist(distLine[Math.abs((y * targetThreads)+threadNumber - ty)]);
            }
        }
        //System.out.println("test " + i + " " + threadNumber);
    }
    callback.alertComplete(this);
    return;
}

the above code is a part of the run method in a Runnable class and it is being executed as a separate thread from the main program.

the code in the alertComplete method called at the end is:

public void alertComplete(SimThread s) {
    completedThreads ++;
    int threadNumber = s.getThreadNumber();
    int targetThreads = s.getTargetThreads();
    double[][] temp = s.getResultMatrix();
    for (int x = 0; x < temp.length; x++)
    {
        for (int y = 0; y < temp[x].length && (y*targetThreads)+threadNumber < tempMatrix[x].length; y++)
        {
            double t = temp[x][y];
            tempMatrix[x][(y*targetThreads)+threadNumber] = t;
        }
    }
    if (completedThreads == MAXTHREADS)
    {
        System.out.println("Calculating points took " + ((System.currentTimeMillis() - startTime)) + " milliseconds.");
        normalizeAndDraw(tempMatrix);
    }

}

the println call in this method does not execute when the println in the above code is commented.

`the code below does not and instead the program produces no output at all` Well, obviously, the second program does not produce output because you removed the `println()` statement, right? — stackoverflowuser2010, Nov 23 '16 at 08:12
`System.out.println` is internally synchronized. That may cause a difference. — Andy Turner, Nov 23 '16 at 08:13
the callback.alertComplete(this) call gives the data back to the main program and when all threads are complete it compiles the data and generates the output to the screen. When the println is commented no output appears on the the screen at all. — user2209743, Nov 23 '16 at 08:17
We have no idea what `callback` is or how its `alertComplete` method works. It would be *much* easier to help you if you'd provide a [mcve]. — Jon Skeet, Nov 23 '16 at 08:18
callback is a custom interface i made for the application simply requiring the source class to have the alertComplete method — user2209743, Nov 23 '16 at 08:21
As you can see [here](http://grepcode.com/file/repository.grepcode.com/java/root/jdk/openjdk/8u40-b25/java/io/PrintStream.java#PrintStream.println%28%29). The print methods use `synhronize`. Which is a possible explanation why a `print` might change how your program works. You should either write a `println("")` or surround that part of the code with a `synchronized(this)`. — nick zoum, Nov 23 '16 at 08:27

user85421 · Accepted Answer · 2016-11-23T08:41:04.910

It is very hard to analyze the problem without being able to run it.

My guess would be to synchronize alertComplete, or better, synchronize the access to completedThreads - eventually the problem is that two or more Threads are updating the counter at the same time so it never reaches MAXTHREADS. Or try using an AtomicInteger for the counter...

But this is just a very rough guess since I haven't got the whole idea...

Assuming that the counter is only used in alertComplete, you could do :

private final Object counterLock = new Object(); // could use any other existing (meaningfull) object

...

public void alertComplete(SimThread s) {
    bollean done;
    synchronized (counterLock) {
        completedThreads ++;
        done = completedThreads == MAXTHREADS;
    }

    ...

    if (done)
    {
        System.out.println("Calculating points took " + ((System.currentTimeMillis() - startTime)) + " milliseconds.");
        normalizeAndDraw(tempMatrix);
    }

}

or just declare the method synchronized (for testing):

public synchronized void alertComplete(SimThread s) {
    ...
}

I think you should add some code to show how it should be done. I don't think OP knows how to do it. You could add `synchronized(this /*or use a syncobject*/ ){ /*your code */ }` — nick zoum, Nov 23 '16 at 08:32
thank you, synchronizing `alertComplete` solved the problem. tbh, i dont know why I didn't run into the problem in earlier versions of the program. — user2209743, Nov 23 '16 at 08:47
big problem of threading: timing can be different every time and some bugs never show up, or only at the worst possible moment — user85421, Nov 23 '16 at 08:49

Removing a println call in java is causing a logical error

1 Answers1