Memory Leak questions, code review

[mensaFool]

Ok, let me start this by saying "I know that Thread.sleep(int) is bad." There are you happy? I am curious if Thread.sleep() is truely the cause of the problem within the application that I created.

Issue - When creating a .jar, sending over to an AIX server, running with java, after about two days, I get a heap dump and a out of mem exception. When I ran this through the eclipse IDE for a few days, it ran flawlessly. Can anyone review or advise on how to approach this differently, why this thing might be leaking memory or eating up all the allocated memory?

Please note - I planned on deploying proper logging with this application but never got that far, I appreciate the advice and review. I have the dmp and txt file that came with the out of mem exception.

package monitor;

import java.io.*;
import java.text.SimpleDateFormat;
import java.util.ArrayList;
import java.util.Calendar;

import org.apache.commons.net.ftp.FTPClient;
import org.apache.commons.net.ftp.FTPFile;

import communication.Communication;
import communication.Email;

/**
 * BillingFileMonitor class is a thread based monitoring system
 * 
 * @author 
 * @version 
 *  
 */
public class BillingFileMonitor implements Runnable {
	private final Thread BMThread;
	String DATE_FORMAT = "yyyyMMdd";
	SimpleDateFormat sdf = new SimpleDateFormat(DATE_FORMAT);

	ArrayList<File> FINFiles, eGateFiles;
	File fileNames, toEmailAddress;
	StringBuffer emailMessage, emailSubject, FINFileNames, eGateFileNames;

	public BillingFileMonitor() {

		BMThread = new Thread(this);
		BMThread.start();
	}

	public void run() {
		fileNames = new File(
				"/BillingConfigFiles/ReportedFileNames.txt");
		toEmailAddress = new File(
				"/BillingConfigFiles/EmailMonitor.txt");
		/*
		fileNames = new File(
		"\\\\BillingConfigFiles\\ReportedFileNames.txt");
		toEmailAddress = new File(
		"\\\\BillingConfigFiles\\RegularTestEmailDaily.txt");
		*/
		eGateFiles = new ArrayList<File>();

		if (!fileNames.exists()) {
			try {
				fileNames.createNewFile();
			} catch (IOException e) {
				// TODO Auto-generated catch block
				e.printStackTrace();
			}
		}
		String fileName;
		ArrayList<String> listOfFileNames = new ArrayList<String>();
		try {
			FileInputStream fileNameInput = new FileInputStream(fileNames);
			DataInputStream Din = new DataInputStream(fileNameInput);
			BufferedReader brr = new BufferedReader(new InputStreamReader(Din));
			while ((fileName = brr.readLine()) != null) {
				listOfFileNames.add(fileName);
				// System.out.println(listOfFileNames.toString());
			}
			Din.close();
			fileNameInput.close();
			brr.close();
		} catch (Exception e) {
			// TODO Auto-generated catch block
			e.printStackTrace();
		}
		final String EGATEPATH = "/bill/takenData";
		try {
			FTPClient ftp = new FTPClient();
			ftp.connect("");
			ftp.login("", "");
			FTPFile[] ftpFiles = ftp.listFiles(EGATEPATH);
			for (FTPFile x : ftpFiles) {
				if (x.getName().contains("HL7_")
						&& !listOfFileNames.contains(x.getName())) {
					File ftemp = new File(x.getName());
					OutputStream fop = new FileOutputStream(ftemp);
					ftp.retrieveFile(EGATEPATH + "/" + x.getName(), fop);
					eGateFiles.add(ftemp);
					fop.close();
				}
			}
			ftp.disconnect();
		} catch (Exception e) {
			System.out.print(e);
		}
		FINFiles = new ArrayList<File>();
		final String FINPATH = "/hl7";
		try {
			FTPClient ftp = new FTPClient();
			ftp.connect("");
			ftp.login("", "");
			FTPFile[] ftpFiles = ftp.listFiles(FINPATH);
			for (FTPFile x : ftpFiles) {
				if (x.getName().contains("HL7ARC_COPATH")
						&& !listOfFileNames.contains(x.getName())) {
					File ftemp = new File(x.getName());
					OutputStream fop = new FileOutputStream(ftemp);
					ftp.retrieveFile(FINPATH + "/" + x.getName(), fop);
					FINFiles.add(ftemp);
					fop.close();
				}
			}
			ftp.disconnect();
		} catch (Exception e) {
			// TODO Auto-generated catch block
			e.printStackTrace();
		}
		final String FINARCHIVEPATH = "/archive";
		try {
			FTPClient ftp = new FTPClient();
			ftp.connect("");
			ftp.login("", "");
			FTPFile[] ftpFiles = ftp.listFiles(FINARCHIVEPATH);
			for (FTPFile x : ftpFiles) {
				if (x.getName().contains("HL7ARC_COPATH")
						&& !listOfFileNames.contains(x.getName())) {
					File ftemp = new File(x.getName());
					OutputStream fop = new FileOutputStream(ftemp);
					ftp.retrieveFile(FINARCHIVEPATH + "/" + x.getName(), fop);
					FINFiles.add(ftemp);
					fop.close();
				}
			}
			ftp.disconnect();
		} catch (Exception e) {
			// TODO Auto-generated catch block
			e.printStackTrace();
		}

		if (eGateFiles.size() > 0 || FINFiles.size() > 0) {
			Email monitorEmail = new Email();
			emailMessage = new StringBuffer();
			emailSubject = new StringBuffer();
			constructEmailSubjectAndMessage();
			if (eGateFiles.size() > 0) {

				emailMessage.append("\n"
						+ "-------------- Start eGate Received --------------"
						+ "\n");
				for (File egatefile : eGateFiles) {
					eGateFileNotFound(egatefile);
				}
				emailMessage.append("\n"
						+ "-------------- End eGate Received --------------"
						+ "\n");
			}
			if (FINFiles.size() > 0) {
				emailMessage.append("\n"
						+ "-------------- Start Fin Received --------------"
						+ "\n");
				for (File finfile : FINFiles) {
					FINFileNotFound(finfile);
				}
				emailMessage.append("\n"
						+ "-------------- End Fin Received --------------"
						+ "\n");
			}

			System.out.print(emailMessage + " ");
			Calendar calendar = Calendar.getInstance();
			int day = calendar.get(Calendar.DAY_OF_MONTH);
		    int month = calendar.get(Calendar.MONTH);
		    int year = calendar.get(Calendar.YEAR);
		    int hour = calendar.get(Calendar.HOUR_OF_DAY);
		    int minute = calendar.get(Calendar.MINUTE);
		    int second = calendar.get(Calendar.SECOND);
		    System.out.print(month + "/" + day + "/" + year + " "+ hour + ":" + minute / 10 + minute % 10 +
		                        ":" + second / 10 + second % 6000);
			sendEmail(monitorEmail);
		} else {
			System.out.println("no new files found ");
			System.out.print(Calendar.MONTH + "/" + Calendar.DATE + "/"+ Calendar.YEAR +
					" " + Calendar.HOUR_OF_DAY + ":" + Calendar.MINUTE);
		}

		try {
			System.out.print(Runtime.getRuntime().freeMemory()+ " bytes free!");
			FINFiles = null;
			eGateFiles = null;
			emailMessage = null;
			emailSubject = null;
			FINFileNames = null;
			eGateFileNames = null;
			BMThread.sleep(900000);
			this.run();
		} catch (Exception e) {
			// TODO Auto-generated catch block
			e.printStackTrace();
		}
	}

	private void constructEmailSubjectAndMessage() {
		FINFileNames = new StringBuffer();
		eGateFileNames = new StringBuffer();
		for (File e : eGateFiles) {
			try {
				FileInputStream fstream = new FileInputStream(e);
				DataInputStream in = new DataInputStream(fstream);
				BufferedReader br = new BufferedReader(
						new InputStreamReader(in));
				String s = br.readLine();
				eGateFileNames.append(s.substring(51, s.length() - 2) + " ");
				fstream.close();
				in.close();
				br.close();
			}// end try
			catch (Exception ex) {// muffle
			}// end catch
		}// end for
		for (File f : FINFiles) {
			try {
				FileInputStream fstream = new FileInputStream(f);
				DataInputStream in = new DataInputStream(fstream);
				BufferedReader br = new BufferedReader(
						new InputStreamReader(in));
				String s = br.readLine();
				FINFileNames.append(s.substring(55, s.length() - 2) + " ");
				fstream.close();
				in.close();
				br.close();
			}// end try
			catch (Exception ex) {// muffle
			}// end catch
		}// end for

		if (FINFiles.size() > 0 && eGateFiles.size() > 0) {
			emailSubject.append("Billing files: eGate("
					+ eGateFileNames.toString() + ") and FIN("
					+ FINFileNames.toString() + ")");
			emailMessage.append("eGate has received " + eGateFiles.size()
					+ " billing file(s). \n");
			emailMessage.append("FIN has received " + FINFiles.size()
					+ " billing file(s). \n");
		}
		if (FINFiles.size() > 0 && eGateFiles.size() < 1) {
			emailSubject.append("Billing file: FIN(" + FINFileNames.toString()
					+ ")");
			emailMessage.append("FIN has received " + FINFiles.size()
					+ " billing file(s). \n");
		}
		if (FINFiles.size() < 1 && eGateFiles.size() > 0) {
			emailSubject.append("Billing files: eGate("
					+ eGateFileNames.toString() + ")");
			emailMessage.append("eGate has received " + eGateFiles.size()
					+ " billing file(s). \n");
		}
	}

	private void sendEmail(Email monitorEmail) {
		// monitorEmail.setToAddress("");
		try {
			FileInputStream fstream = new FileInputStream(toEmailAddress);
			DataInputStream in = new DataInputStream(fstream);
			BufferedReader br = new BufferedReader(new InputStreamReader(in));
			String s = br.readLine();
			fstream.close();
			in.close();
			br.close();
			monitorEmail.setToAddress(s);
			monitorEmail.setSubject(emailSubject.toString());
			monitorEmail.setMessage(emailMessage.toString());
			Communication c = Communication.getInstance();
			c.sendEmail(monitorEmail);
			
		} catch (Exception e) {
			e.printStackTrace();
		}

	}

	private void eGateFileNotFound(File f) {
		BufferedWriter bufferedWriter = null;
		try {
			bufferedWriter = new BufferedWriter(new FileWriter(fileNames, true));
		} catch (IOException e1) {
			// TODO Auto-generated catch block
			e1.printStackTrace();
		}
		try {
			FileInputStream fstream = new FileInputStream(f);
			DataInputStream in = new DataInputStream(fstream);
			BufferedReader br = new BufferedReader(new InputStreamReader(in));
			String s = br.readLine();
			// read only the first line.
			// Date fileDate = new Date(f.lastModified());
			emailMessage.append("\n"
					+ s.substring(51, s.length() - 2)
					+ " @ "
					+ f.getName().substring(f.getName().length() - 10,
							f.getName().length() - 8)
					+ "/"
					+ f.getName().substring(f.getName().length() - 8,
							f.getName().length() - 6)
					+ " "
					+ f.getName().substring(f.getName().length() - 6,
							f.getName().length() - 2) + "\n");
			emailMessage.append(s + "\n");
			bufferedWriter.write(f.getName());
			bufferedWriter.newLine();
			fstream.close();
			br.close();
			in.close();
		} catch (Exception e) {
		} finally {
			try {
				if (bufferedWriter != null) {
					bufferedWriter.flush();
					bufferedWriter.close();
					f.delete();
				}
			} catch (Exception ex) {
				ex.printStackTrace();
			}
		}
	}

	private void FINFileNotFound(File finfile) {
		BufferedWriter bufferedWriter = null;
		try {
			bufferedWriter = new BufferedWriter(new FileWriter(fileNames, true));
		} catch (IOException e1) {
			// TODO Auto-generated catch block
			e1.printStackTrace();
		}
		try {
			FileInputStream fstream = new FileInputStream(finfile);
			DataInputStream in = new DataInputStream(fstream);
			BufferedReader br = new BufferedReader(new InputStreamReader(in));
			String s = br.readLine();

			// read only the first line.
			// Date fileDate = new Date(f.lastModified());
			emailMessage.append("\n" + s.substring(55, s.length() - 2) + " @ "
					+ finfile.getName().substring(18, 20) + "/"
					+ finfile.getName().substring(20, 22) + "\n");
			emailMessage.append(s + "\n");
			bufferedWriter.write(finfile.getName());
			bufferedWriter.newLine();
			fstream.close();
			br.close();
			in.close();
		} catch (Exception e) {
		} finally {
			try {
				if (bufferedWriter != null) {
					bufferedWriter.flush();
					bufferedWriter.close();
					finfile.delete();
				}
			} catch (Exception ex) {
				ex.printStackTrace();
			}
		}

	}

}

[mensaFool]

I should say also after posting that huge amount of code that I intended upon altering the design patterns. Also, could this be a garbage collection issue? Is there a better way to release the resources? Is that the problem?

[Eranga]

All the unnecessary memory de-allocation done by the garbage collector automatically for you in java. But the thing we have to keep in mind is that it's not 100% guaranteed it'll work properly. Sometimes our implementation can cause problems. Specially when work on with binary streams and stuff. Make sure in such cases safely close down after the process.

And the other point is that once you run your IDE for a long time, how it will effect? May be the cause is not your application. You have to consider about that as well.

And also, please be specific with your question. I mean I don't think no one ant to run your code for two days and comment on it.

[toadaly]

The usual culprits of Java memory leaks are:

- collections that grow unbounded (it doesn't look like you're doing that)
- system resources that are not getting freed. For example, a failure to call 'dispose' on a graphics context you are creating over and over (it doesn't look like you're doing any of that either)

Since there are no apparent culprits, it may simply be that the garbage collector is not being aggressive enough and your memory usage temporarily peaks above the maximum heap size. You can usually fix that by:

- explicitly increasing the maximum heap size using the -Xmx java switch
- embedding a call to System.gc() after you dereference significant chunkc of memory. Try putting that into your try-catch block where you're setting everything to null, after you set them to null.

[Eranga]

Try putting that into your try-catch block where you're setting everything to null, after you set them to null.

This is a very good point. Basically clean-up all the unnecessary initializations make sens in most of the cases.

[mensaFool]

Thanks for looking over the code and giving it a shot. I will implement this tomorrow and see where it goes from there. I may increase the timer to 1 hour as well to see if that increases the garbage collection.

[mensaFool]

try {
			 System.out.print(month + "/" + day + "/" + year + " "+ hour + ":" + minute / 10 + minute % 10 +
                     ":" + second / 10 + second % 6000);
			System.out.print("\n" + Runtime.getRuntime().freeMemory()+ " bytes free!");
			System.out.print("\n" + Runtime.getRuntime().maxMemory()+ " max Memory!");
			System.out.print("\n" + Runtime.getRuntime().totalMemory()+ " total Memory!");
			FINFiles = null;
			eGateFiles = null;
			emailMessage = null;
			emailSubject = null;
			FINFileNames = null;
			eGateFileNames = null;
			System.gc();
			BMThread.sleep(900000);
			this.run();
		} catch (Exception e) {
			// TODO Auto-generated catch block
			e.printStackTrace();
		}
	}

Yielded:
no new files found
2/5/2010 12:16:554
1300448 bytes free!
419430400 max Memory!
4194304 total Memory!

[Eranga]

Now what you think?

[mensaFool]

-Xmx 400m and System.gc solved it. I think it was more the System.gc() more than anything. It doesn't even consume more than the first block that's allocated. Thanks for all the help guys!


no new files found
2/7/2010 19:30:558
405464 bytes free!
419430400 max Memory!
4194304 total Memory!
eGate has received 1 billing file(s).

-------------- Start eGate Received --------------

I @ 03/07 1945
FHS|^~\&|||

-------------- End eGate Received --------------
2/7/2010 19:45:558
106192 bytes free!
419430400 max Memory!
4194304 total Memory!
no new files found
2/7/2010 20:01:00
380728 bytes free!
419430400 max Memory!
4194304 total Memory!
eGate has received 1 billing file(s).

-------------- Start eGate Received --------------

A @ 03/07 2015
FHS|^~\&|

-------------- End eGate Received --------------
2/7/2010 20:16:00
94880 bytes free!
419430400 max Memory!
4194304 total Memory!
no new files found
2/7/2010 20:31:111
378800 bytes free!
419430400 max Memory!
4194304 total Memory!
eGate has received 1 billing file(s).

-------------- Start eGate Received --------------

A @ 03/07 2045
FHS|^~\&|

-------------- End eGate Received --------------
2/7/2010 20:46:112
75776 bytes free!
419430400 max Memory!
4194304 total Memory!
no new files found
2/7/2010 21:01:223
372152 bytes free!
419430400 max Memory!
4194304 total Memory!

[Eranga]

Cool. If you've the problem then mark the thread solved from the tools menu.