Skip to content

Conversation

@adamjohnwright
Copy link

Fixed 5 instances of improper stream management that could cause CLOSE_WAIT connections in Apache when exceptions occur during file export operations.

Changes:

  • PptxExporterController: Wrapped FileInputStream in try-with-resources for both diagramPPTX() and reactionPPTX() methods
  • SbxxExporterController: Wrapped InputStream in try-with-resources for both eventSBGN() and eventSBML() methods
  • CitationController: Wrapped InputStream in try-with-resources for exportCitation() method

Previously, if IOUtils.copy() or flush() threw an exception, streams would not be closed, leaving file descriptors and HTTP connections in CLOSE_WAIT state. The try-with-resources pattern ensures proper cleanup even when exceptions occur.

Note: HttpServletResponse.getOutputStream() is intentionally not closed as the servlet container manages its lifecycle.

🤖 Generated with Claude Code (https://claude.com/claude-code)

@jweiser and I created this and compiled it, but did not run it. We are trying to fix the open Apache2 connections. We know you are on vacation. If you are not able to take a look, could you at least let us know if what is on prod is from the latest commit on the Master branch? And would it be safe to make this change inside and test it first, making sure it still runs?

Fixed 5 instances of improper stream management that could cause
CLOSE_WAIT connections in Apache when exceptions occur during file
export operations.

Changes:
- PptxExporterController: Wrapped FileInputStream in try-with-resources
  for both diagramPPTX() and reactionPPTX() methods
- SbxxExporterController: Wrapped InputStream in try-with-resources
  for both eventSBGN() and eventSBML() methods
- CitationController: Wrapped InputStream in try-with-resources
  for exportCitation() method

Previously, if IOUtils.copy() or flush() threw an exception, streams
would not be closed, leaving file descriptors and HTTP connections
in CLOSE_WAIT state. The try-with-resources pattern ensures proper
cleanup even when exceptions occur.

Note: HttpServletResponse.getOutputStream() is intentionally not closed
as the servlet container manages its lifecycle.

🤖 Generated with Claude Code (https://claude.com/claude-code)

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@EliotRagueneau
Copy link
Contributor

It seems Chuqiao has already merged all the canges which more mostly located in feature/include-refs-with-JSOG in dev and main. However I noticed that someone deleted the remote dev branch, that's doesn't look good :/

Otherwise for this PR specifically, it looks okay to me what Claude did. as for AnalysisService, you should first try it on dev derver to check that it works without breaking anything before deploying onto produciton, but in principle I think it really shouldn't create an issue.

@adamjohnwright
Copy link
Author

@EliotRagueneau Just like the other pr from the analysis service, @jweiser and I will try to test this on dev as soon as possible. Thanks for taking a look.

As for the dev branch, we have been deleting dev branches when they are identical to the main branch. I don't remember for sure with this repo, but I think I remember deleting it just before the break. We have had issues with knowing the status of the dev branch (what is on it) relative to the main branch. This especially becomes difficult when working with over a hundred repos. I would prefer to just pull branches into the main branch when they have been fully tested. This was agreed upon during a dev call six months or a year ago. We mainly made the change to the repos that we work with at OICR. Right now, I try to remove it when working on cleaning up branches for a particular repo we were working on. Hopefully, this works for you.

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

Successfully merging this pull request may close these issues.

3 participants