2

Currently I am dealing with my first memory leak, and I can't seem to fix it. My program is supposed to play .wav audio files, via a class called JukeBox. JukeBox works fine but each time I play a new sound, the memory amount that java is using rises in Task manager and once I hit ~316,500K I receive the following error in the System.err terminal:

Exception in thread "Thread-6" java.lang.OutOfMemoryError: Java heap space
at com.sun.media.sound.DirectAudioDevice$DirectClip.open(DirectAudioDevice.java:1135)
at JukeBox.play(JukeBox.java:53)
at Instrument.play(Instrument.java:65)
at Conductor.play(Conductor.java:78)
at Game$2.run(Game.java:42)

In my debugger, thread 6 is one of the two threads that constantly look for keyboard input (specifically the one that triggers the sound).

Here is a screenshot of my debugger after the error: image from debugger of threads

so I am fairly sure that my problem is caused by having too many open clip threads, but that's where I am really confused, because I thought I was closing them, when in reality it looks like they're just hanging there, waiting.

Here is my code for where I think I am closing the clip:

public void play(int pitch){
    jukez = new JukeBox("pianoNote.wav");

    Thread clipkill = new Thread() {
        public void run() {
            try {
                Thread.sleep(jukez.getMicrosecondLength()*1000);
            } catch (Exception e){System.out.println(e);}
            jukez.killTheTunes();
        }
    };
    jukez.play();
    clipkill.start();
}

and JukeBox is here:

public class JukeBox{
    private AudioInputStream stream;
    private AudioFormat format;
    private DataLine.Info info;
    private Clip clip;
    private String name;

    /**
     * Constructor for objects of the class Jukebox
     * @param str the filename of the wave file to pla
     */
    public JukeBox(String str){
        try{
            stream = AudioSystem.getAudioInputStream(new File(str));
            name = str;
            format = stream.getFormat();
            info = new DataLine.Info(Clip.class, format);
            clip = (Clip) AudioSystem.getLine(info);
        } catch(Exception E){
            System.out.println(E);
        }
    }

    /**
     * Returns the clip's length in microseconds.
     * @return the clip's length in microseconds.
     */
    public long getMicrosecondLength(){return clip.getMicrosecondLength();}

    /**
     * Plays the music on its own thread, enabling the game to run while music plays
     */
    public void play(){
        try {
            clip.open(stream);
            clip.start();
        }
        catch (Exception e) {
            System.out.println(e);
        }
    }

    /**
     * Ends the music and it's thread as well
     */
    public void killTheTunes(){
        try{stream.close();}catch (Exception e) {
            System.out.println(e);
        }
        clip.close();
    }

    /**
     * Returns the filename the jukebox was initially set to play
     * @return the filename the jukebox was initially set to play
     */
    public String toString(){
        return name;
    }
}

I am not sure why I still am having heap memory issues. Thank you

Edit:

The structure of my code is a bit complex, so I will try to explain it. The main method to run the game looks like:

    display = new Display();
    copper = new Conductor();
    display.main(); //sets up all the jframe things and graphics
    Thread p1 = new Thread() {
            public void run() {
                while(true){
                    if(display.panel.playerClicked(1)){
                        display.panel.p1 = Color.black;
                        display.panel.p1 = copper.play(4,copper.determinePitch(display.panel.getY(1)));
                        display.panel.pause(250);
                        display.panel.p1 = Color.black;
                    }
                }
            }
        };

    Thread p2 = new Thread() {
            public void run() {
                while(true){
                    if(display.panel.playerClicked(2)){
                        display.panel.p2 = Color.black;
                        display.panel.p2 = copper.play(0,copper.determinePitch(display.panel.getY(2)));
                        display.panel.pause(250);
                        display.panel.p2 = Color.black;
                    }
                }
            }
        };

    p1.start();
    p2.start();

Here is the Canvas class, which detects key movements. Display basically just initializes a JFrame and adds a Canvas to it.

import java.awt.*;
import java.awt.geom.*;
import java.awt.event.*;
import javax.swing.JPanel;
import java.util.ArrayList;
import java.awt.image.BufferedImage;
import java.io.*;
import javax.imageio.ImageIO;
import java.awt.Image;
import javax.swing.JLabel;
import javax.swing.ImageIcon;


public class Canvas extends JPanel {
    // attributes
    private Rectangle player1;
    private Rectangle player2;

public final int x = 800 + 200;
public final int y = 1100/2 + 200;

private boolean[] player1bools = new boolean[5];
private boolean[] player2bools = new boolean[5];
BufferedImage dimg;

Color p1 = Color.black; //these are intentionally public so game can change them
Color p2 = Color.black;

// constructor
public Canvas() {
    // initialize object
    player1 = new Rectangle(250, 50, 50, 50);
    player2 = new Rectangle(50, 50, 50, 50);

    // set canavs background colour

    // add the key listener in the constructor of your canavas/panel
    addKeyListener(new myKeyListener());


    BufferedImage img = null;
    try {
        img = ImageIO.read(new File("Pencils.jpg"));
    } catch (IOException e) {}
    dimg = resize(img, x, y-20);

    // ensure focus is on this canavas/panel for key operations.
    setFocusable(true);
    setBackground(new Color(0,0,0,0));
    setOpaque(true);

    Thread gameLoop = new Thread() {
            public void run() {
                while(true){
                    updatePlayers();
                    repaint();

                    pause(20);
                }

            }
        };

    gameLoop.start();

}

public int getY(int i){
    if(i == 1){return player1.y;}
    if(i == 2){return player2.y;}
    return -1;
}

public boolean isOptimizedDrawingEnabled(){return false;}

/**
 * This method was made by David Kroukamp on
 * http://stackoverflow.com/questions/14548808/scale-the-imageicon-automatically-to-label-size
 */
public static BufferedImage resize(BufferedImage image, int width, int height) {
    BufferedImage bi = new BufferedImage(width, height, BufferedImage.TRANSLUCENT);
    Graphics2D g2d = (Graphics2D) bi.createGraphics();
    g2d.addRenderingHints(new RenderingHints(RenderingHints.KEY_RENDERING, RenderingHints.VALUE_RENDER_QUALITY));
    g2d.drawImage(image, 0, 0, width, height, null);
    g2d.dispose();
    return bi;
}

private void updatePlayers() {
    if (player1bools[0]) {
        moveX(-5, player1);
    }
    if (player1bools[2]) {
        moveX(5, player1);
    }
    if (player1bools[1]) {
        moveY(-5, player1);
    }
    if (player1bools[3]) {
        moveY(5, player1);
    }

    if (player2bools[0]) {
        moveX(-5, player2);
    }
    if (player2bools[2]) {
        moveX(5, player2);
    }
    if (player2bools[1]) {
        moveY(-5, player2);
    }
    if (player2bools[3]) {
        moveY(5, player2);
    }
}

// painting
public void paintComponent(Graphics graphics) {
    super.paintComponent(graphics);
    graphics.clearRect(0, 0, getWidth(), getHeight()); //clears the trails

    graphics.drawImage(dimg,0,0,null);
    Graphics2D graphics2d = (Graphics2D) graphics;
    graphics.setColor(p1);
    graphics2d.fill(player1);
    graphics.setColor(p2);
    graphics2d.fill(player2);
    // */
}

// function which essentially re-creates rectangle with varying x
// orientations. (x-movement)
public void moveX(int mutationDistance, Rectangle sampleObject) {
    //I don't want horizontal movement to work

    /*
    sampleObject.setBounds(sampleObject.x + mutationDistance,
    sampleObject.y, sampleObject.width, sampleObject.height);
     */

}

// function which essentially re-creates rectangle with varying y
// orientations. (y-movement)
public void moveY(int mutationDistance, Rectangle sampleObject) {
    if(sampleObject.y < 0) {
        sampleObject.y = 0;
    }
    else if(sampleObject.y  > 670) {
        sampleObject.y = 670;
    }

    sampleObject.setBounds(sampleObject.x, sampleObject.y
        + mutationDistance, sampleObject.width, sampleObject.height);
}

public boolean playerClicked(int i){
    if(i == 1){return player1bools[4];} 
    if(i == 2){return player2bools[4];}
    return false;
}

// listener
private class myKeyListener implements KeyListener {
    // implement all the possible actions on keys
    public void keyPressed(final KeyEvent keyEvent) {
        if (keyEvent.getKeyCode() == KeyEvent.VK_ESCAPE) {
            System.exit(0);
        }

        if (keyEvent.getKeyCode() == KeyEvent.VK_RIGHT) {
            player1bools[2] = true;
        }
        if (keyEvent.getKeyCode() == KeyEvent.VK_LEFT) {
            player1bools[0] = true;
        }
        if (keyEvent.getKeyCode() == KeyEvent.VK_UP) {
            player1bools[1] = true;
        }
        if (keyEvent.getKeyCode() == KeyEvent.VK_DOWN) {
            player1bools[3] = true;
        }
        if (keyEvent.getKeyCode() == KeyEvent.VK_SLASH) {
            player1bools[4] = true;
        }

        if (keyEvent.getKeyCode() == KeyEvent.VK_D) {
            player2bools[2] = true;
        }
        if (keyEvent.getKeyCode() == KeyEvent.VK_A) {
            player2bools[0] = true;
        }
        if (keyEvent.getKeyCode() == KeyEvent.VK_W) {
            player2bools[1] = true;
        }
        if (keyEvent.getKeyCode() == KeyEvent.VK_S) {
            player2bools[3] = true;
        }
        if (keyEvent.getKeyCode() == KeyEvent.VK_F) {
            player2bools[4] = true;
        }

    }

    public void keyReleased(KeyEvent keyEvent) {
        if (keyEvent.getKeyCode() == KeyEvent.VK_RIGHT) {
            player1bools[2] = false;
        }
        if (keyEvent.getKeyCode() == KeyEvent.VK_LEFT) {
            player1bools[0] = false;
        }
        if (keyEvent.getKeyCode() == KeyEvent.VK_UP) {
            player1bools[1] = false;
        }
        if (keyEvent.getKeyCode() == KeyEvent.VK_DOWN) {
            player1bools[3] = false;
        }
        if (keyEvent.getKeyCode() == KeyEvent.VK_SLASH) {
            player1bools[4] = false;
        }

        if (keyEvent.getKeyCode() == KeyEvent.VK_D) {
            player2bools[2] = false;
        }
        if (keyEvent.getKeyCode() == KeyEvent.VK_A) {
            player2bools[0] = false;
        }
        if (keyEvent.getKeyCode() == KeyEvent.VK_W) {
            player2bools[1] = false;
        }
        if (keyEvent.getKeyCode() == KeyEvent.VK_S) {
            player2bools[3] = false;
        }
        if (keyEvent.getKeyCode() == KeyEvent.VK_F) {
            player2bools[4] = false;
        }
    }

    public void keyTyped(KeyEvent keyEvent) {
    }
}

public static void pause(int secs) {
    try {
        Thread.sleep(secs);
    } catch (Exception e) {}
}
}

And lastly there is Conductor which manages the instruments here:

public class Conductor
{
private ArrayList<Instrument> symphony;

/**
 * Constructor for objects of class Conductor, initializes the instruments
 */
public Conductor()
{
    ArrayList<String> pianoList = new ArrayList<String>();
    for(int i = 1; i <= 11; i++){
        pianoList.add("piano"+i);
    }

    ArrayList<String> bassList = new ArrayList<String>();
    for(int i = 1; i <= 11; i++){
        bassList.add("bass"+i);
    }

    ArrayList<String> guitarList = new ArrayList<String>();
    for(int i = 1; i <= 11; i++){
        guitarList.add("guitar"+i);
    }

    ArrayList<String> percList = new ArrayList<String>();
    for(int i = 1; i <= 11; i++){
        percList.add("perc"+i);
    }

    ArrayList<String> vibeList = new ArrayList<String>();
    for(int i = 1; i <= 11; i++){
        vibeList.add("vibe"+i);
    }

    Instrument piano = new Instrument(pianoList, new Color(19,130,206));
    Instrument bass = new Instrument(bassList, new Color(100,83,38));
    Instrument guitar = new Instrument(guitarList, new Color(244,184,16));
    Instrument perc = new Instrument(percList, new Color(24,57,165));
    Instrument vibe = new Instrument(vibeList, new Color(57,24,135));

    symphony = new ArrayList<Instrument>();
    symphony.add(piano);
    symphony.add(bass);
    symphony.add(guitar);
    symphony.add(perc);
    symphony.add(vibe);
}

public static int determinePitch(int i){
    if(i == -1){return i;}
    if(i>600){return 1;}
    if(i>535){return 2;}
    if(i>470){return 3;}
    if(i>410){return 4;} //these numbers correspond to the approximate heights of the colors in the background image
    if(i>345){return 5;}
    if(i>275){return 6;}
    if(i>215){return 7;}
    if(i>155){return 8;}
    if(i>90){return 9;}
    if(i>25){return 10;}
    return 11;
}

/**
 * Calls an instrument and makes it play a noise
 * @param inst the instrument index to play
 * @param pitch the pitch for the instrument to play
 */
public Color play(int inst, int pitch){
    symphony.get(inst).play(pitch);
    return symphony.get(inst).getColor();
}
}

So for an overall run down of the structure:

The threads controlling the player detect whether their respective player has hit the trigger key. If so, they call the conductor to play an instrument. They also change the color of that player's rectangle. When the conductor plays an instrument, it uses Instrument's play method, which calls upon the instrument's JukeBox method which uses clips which causes a crash.

ADC2000
  • 63
  • 7
  • Instead of making a new thread for every sound, consider using a [single thread managed by a ScheduledExecutorService](https://docs.oracle.com/javase/8/docs/api/java/util/concurrent/Executors.html#newSingleThreadScheduledExecutor--). – VGR May 15 '16 at 03:25
  • I am not intentionally making a new thread for every sound, I believe that that is just an unfortunate side effect of the Clip class – ADC2000 May 15 '16 at 03:37
  • It sure looks like you’re making a new thread every time play(int) is called. – VGR May 15 '16 at 03:59
  • 1
    Possible duplicate of [java.lang.OutOfMemoryError: Java heap space](http://stackoverflow.com/questions/1596009/java-lang-outofmemoryerror-java-heap-space) – Aditya W May 15 '16 at 04:58
  • Who's calling `play(int)`? – shmosel May 15 '16 at 05:51
  • show us the part of your code that calls play(int) for your test – Nicolas Filotto May 15 '16 at 11:41

2 Answers2

3

Thread.sleep() takes milliseconds as its argument, so the line:

Thread.sleep(jukez.getMicrosecondLength()*1000);

should be:

Thread.sleep(jukez.getMicrosecondLength()/1000);

There may also be other problems in your code, but at present, you are never getting to the killTheTunez() call since you are waiting a million times too long.

Warren Dew
  • 8,790
  • 3
  • 30
  • 44
0

So I fixed my problem. There are some pros and cons to it but overall it works. Here is what I did.

I changed my play method in Instrument so it wasn't creating a new JukeBox each time, but rather Instrument has a JukeBox[] holding the JukeBoxes. I originally was recreating them because I could only play something once, but I figured out that was because I had to move the Clip's read frame back to 0. So now the play method is simple, it is this:

    public void play(int pitch){
    sounds[pitch-1].clip.setFramePosition(0);
    sounds[pitch-1].play();
}

where sounds is a JukeBox[] holding the noises.

It is a bit laggy, and sounds sequentially are a bit clipped. To fix this, I took the hacky approach of making the player rectangle move slower, plus increasing the game's pause time so less pitches can be player as fast, giving time for them to close. On the bright side, no more memory leaks.

ADC2000
  • 63
  • 7