Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

csvReader.close() potential memory leak #58

Open
tttzach opened this issue Jun 22, 2020 · 0 comments
Open

csvReader.close() potential memory leak #58

tttzach opened this issue Jun 22, 2020 · 0 comments

Comments

@tttzach
Copy link
Owner

tttzach commented Jun 22, 2020

Food for thought: if the code which processes the file throws an exception, then you will never call csvReader.close(). This can cause your code to leak file handles and other resources, which would be problematic in a real application with a high volume of requests.

Normally you would handle this by writing something like:

CSVReaderHeaderAware csvReader = null;
try {
  csvReader = ...
  // process the file
} catch (IOException ..) {
  // file doesn't exist (although it could be another error like access denied)
} catch (/* other errors thrown by CSV library */) {
} finally {
  if (csvReader != null) {
    csvReader.Close();
  }
}

Fortunately, modern Java has better syntax for it, but I don't know what version app engine uses. See this article: https://www.infoq.com/news/2010/08/arm-blocks/

try(CSVReaderHeaderAware csvReader = new CSVReaderHeaderAware(new FileReader(filePath))) {
  // process the file
  // no need to close it manually
} catch (IOException /* or whatever */) {
  // ...
}

Originally posted by @antmar in #48

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant