[
https://issues.apache.org/jira/browse/QPID-8550?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17393845#comment-17393845
]
ASF GitHub Bot commented on QPID-8550:
--------------------------------------
alex-rufous commented on a change in pull request #103:
URL: https://github.com/apache/qpid-broker-j/pull/103#discussion_r683282742
##########
File path:
broker-core/src/main/java/org/apache/qpid/server/store/JsonFileConfigStore.java
##########
@@ -156,49 +156,52 @@ protected boolean load(final ConfiguredObjectRecord...
initialRecords)
boolean updated = false;
Collection<ConfiguredObjectRecord> records =
Collections.emptyList();
- ConfiguredObjectRecordConverter configuredObjectRecordConverter =
+ final ConfiguredObjectRecordConverter
configuredObjectRecordConverter =
new ConfiguredObjectRecordConverter(_parent.getModel());
- records = configuredObjectRecordConverter.readFromJson(_rootClass,
_parent, new FileReader(configFile));
-
- if(_rootClass == null)
+ try (FileReader configFileReader = new FileReader(configFile))
Review comment:
This change is unnecessary. The `configFileReader` is closed in
`ObjectMapper`. If you really need to make sure that `configFileReader` is
closed in the code where it is created, you can simply add `try-with-resource`
block around the code reading from json
```
try (FileReader configFileReader = new FileReader(configFile))
{
records = configuredObjectRecordConverter.readFromJson(_rootClass,
_parent, configFileReader);
}
```
##########
File path:
broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/servlet/rest/RestServlet.java
##########
@@ -437,7 +437,10 @@ public String getRequestURL()
{
if ("data".equals(part.getName()) &&
"application/json".equals(part.getContentType()))
{
- items = mapper.readValue(part.getInputStream(),
LinkedHashMap.class);
+ try (InputStream inputStream = part.getInputStream())
Review comment:
The change is unnecessary as methods `ObjectMapper#readValue ` close the
input stream/reader..
##########
File path:
perftests/src/main/java/org/apache/qpid/disttest/controller/config/ConfigReader.java
##########
@@ -37,10 +37,10 @@
public Config getConfigFromFile(String fileName) throws IOException
{
- Reader reader = getConfigReader(fileName);
-
- Config config = readConfig(reader);
- return config;
+ try (Reader reader = getConfigReader(fileName))
Review comment:
The reader is closed in `ObjectMapper#read...`. Thus, the change is
unnecessary...
##########
File path: broker-core/src/main/java/org/apache/qpid/server/util/FileUtils.java
##########
@@ -205,8 +205,10 @@ public static void copy(File src, File dst)
*/
public static void copyCheckedEx(File src, File dst) throws IOException
{
- InputStream in = new FileInputStream(src);
- copy(in, dst);
+ try (InputStream in = new FileInputStream(src))
Review comment:
The `InputStream` is closed in `copy`. Arguably the method `copy` should
indicate that it closes the input stream. It make more sense to rename it into
`copyInputStreamIntoFileAndClose`. Though, I suppose for a better code
relaibility the 'try-with-resource` would be good here.
##########
File path:
broker-core/src/main/java/org/apache/qpid/server/virtualhostnode/AbstractVirtualHostNode.java
##########
@@ -430,40 +430,42 @@ public PreferenceStore createPreferenceStore()
protected final ConfiguredObjectRecord[] getInitialRecords() throws
IOException
{
- ConfiguredObjectRecordConverter converter = new
ConfiguredObjectRecordConverter(getModel());
+ final ConfiguredObjectRecordConverter converter = new
ConfiguredObjectRecordConverter(getModel());
- Collection<ConfiguredObjectRecord> records =
- new
ArrayList<>(converter.readFromJson(VirtualHost.class,this,getInitialConfigReader()));
-
- if(!records.isEmpty())
+ try (final Reader initialConfigReader = getInitialConfigReader())
Review comment:
Try-with-resource block is unnecessary here as reading code is closing
the reader. If you really have to change to `try-with-resource`, it would be
preferable to keep the `try` block small and only use it for reading the
records. IMHO, that makes code more readable
```
final Collection<ConfiguredObjectRecord> records;
try (final Reader initialConfigReader = getInitialConfigReader())
{
records = new ArrayList<>(converter.readFromJson(VirtualHost.class,
this, initialConfigReader));
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
> [Broker-J] HP Fortify: Unreleased streams
> -----------------------------------------
>
> Key: QPID-8550
> URL: https://issues.apache.org/jira/browse/QPID-8550
> Project: Qpid
> Issue Type: Improvement
> Components: Broker-J
> Affects Versions: qpid-java-broker-8.0.5
> Reporter: Daniil Kirilyuk
> Priority: Minor
>
> HP Fortify complains about several places where streams potentially do not
> get released properly:
> broker-core/src/main/java/org/apache/qpid/server/store/JsonFileConfigStore.java
> The function load() in JsonFileConfigStore.java sometimes fails to release a
> system resource allocated by FileReader() on line 162.
> broker-core/src/main/java/org/org/apache/qpid/server/virtualhostnode/AbstractVirtualHostNode.java
> The function getInitialRecords() in AbstractVirtualHostNode.java sometimes
> fails to release a system resource allocated by ,[object Object], on line 435.
> broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/servlet.rest/RestServlet.java
> The function parse() in RestServlet.java sometimes fails to release a system
> resource allocated by getInputStream() on line 440.
> tools/src/main/java/org/apache/qpid/tools/RestStressTestClient.java
> The function put() in RestStressTestClient.java sometimes fails to release a
> system resource allocated by getOutputStream() on line 318.
> perftests/src/main/java/org/apache/qpid/disttest/controller/config/ConfigReader.java
> The function getConfigFromFile() in ConfigReader.java sometimes fails to
> release a system resource allocated by getConfigReader() on line 40.
> perftests/src/main/java/org/apache/qpid/disttest/controller/config/JavaScriptConfigEvaluator.java
> The function evaluateJavaScript() in JavaScriptConfigEvaluator.java sometimes
> fails to release a system resource allocated by InputStreamReader() on line
> 70.
> The function evaluateJavaScript() in JavaScriptConfigEvaluator.java sometimes
> fails to release a system resource allocated by InputStreamReader() on line
> 71.
>
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
