Skip to content

Conversation

@artem-smotrakov
Copy link
Contributor

ContentSigner objects hold an output stream that may need to be closed to prevent resource leaks. A caller can get the stream by calling getOutputStream() method, and close it once the work is done. However, it may be easy to forget to do that.

If ContentSigner extended AutoCloseable:

  • a caller could use it in a try-with-resources block
  • static analysis tools could detect places where a ContentSigner.close() is not called

Updating ContentSigner to extend AutoCloseable is definitely an update in the public API. That may potentially result to compilation errors in client apps. Those errors should be easy to fix thought. Moreover, those errors may make developers look at and fix possible resource leaks in their code.

Here is a summary of the update:

  • Updated interface ContentSigner to extend AutoCloseable.
  • Implemented close() method in several anonymous classes that implement ContentSigner.

- Updated interface ContentSigner to extend AutoCloseable
- Implemented close() method in several anonymous classes
  that implement ContentSigner
@dghgit
Copy link
Contributor

dghgit commented Nov 20, 2020

I'm not sure... was there a specific reason you did not suggest changing the return for getOutputStream()?

@artem-smotrakov
Copy link
Contributor Author

Do you mean changing the return type of getOutputStream()? I don't think it's going to help. The main idea here is to let ContentSigner close its resources by itself in its close() method. Then, allers are only responsible to close the ContentSigner and it then does all the required work by itself.

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.

2 participants