You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by GitBox <gi...@apache.org> on 2022/09/19 10:09:19 UTC

[GitHub] [commons-vfs] destroydestiny opened a new pull request, #300: VFS-824 HttpFileSystem free Unused Resources lead to HttpClient Conn…

destroydestiny opened a new pull request, #300:
URL: https://github.com/apache/commons-vfs/pull/300

   …ection Pool Shutdown


-- 
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: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [commons-vfs] garydgregory commented on a diff in pull request #300: VFS-824 HttpFileSystem free Unused Resources lead to HttpClient Conn…

Posted by GitBox <gi...@apache.org>.
garydgregory commented on code in PR #300:
URL: https://github.com/apache/commons-vfs/pull/300#discussion_r976567939


##########
commons-vfs2/src/main/java/org/apache/commons/vfs2/provider/AbstractFileProvider.java:
##########
@@ -137,17 +137,28 @@ protected FileSystem findFileSystem(final Comparable<?> key, final FileSystemOpt
      * Frees unused resources.
      */
     public void freeUnusedResources() {
+        // process snapshot outside lock
+        Stream.of(getAllFileSystemSnapshot()).filter(AbstractFileSystem::isReleaseable)
+                                      .forEach(AbstractFileSystem::closeCommunicationLink);
+    }
+
+
+    /**
+     * Gets snapshot of all fileSystem under lock.
+     *
+     * @return AbstractFileSystem Array
+     * @since  2.10.0

Review Comment:
   Remove extra whitespace after since tag



-- 
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: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [commons-vfs] garydgregory commented on pull request #300: VFS-824 HttpFileSystem free Unused Resources lead to HttpClient Conn…

Posted by GitBox <gi...@apache.org>.
garydgregory commented on PR #300:
URL: https://github.com/apache/commons-vfs/pull/300#issuecomment-1397014194

   I need to look at this more closely over the weekend: I don't know why the HTTP providers have to be unique and different compared to all the others. Is the use of concepts in this PR backward? In the PR, the "free" code now also "closes" resources and that feels backward to me. I expect the "close" code to also "free" resources are part of closing, not the other way around.
   Any thoughts?


-- 
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: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [commons-vfs] garydgregory commented on a diff in pull request #300: VFS-824 HttpFileSystem free Unused Resources lead to HttpClient Conn…

Posted by GitBox <gi...@apache.org>.
garydgregory commented on code in PR #300:
URL: https://github.com/apache/commons-vfs/pull/300#discussion_r976574200


##########
commons-vfs2/src/test/java/org/apache/commons/vfs2/provider/http5/Http5ProviderTestCase.java:
##########
@@ -210,4 +213,36 @@ public void testResolveFolderSlashYesRedirectOn() throws FileSystemException {
         testResolveFolderSlash(ConnectionUri + "/read-tests/", true);
     }
 
+    @Test
+    public void testHttp5FileSystemFreeUnusedResources() throws Exception {
+        try (StandardFileSystemManager fileSystemManager = new StandardFileSystemManager()) {
+            fileSystemManager.setConfiguration(StandardFileSystemManager.class.getResource("providers.xml"));
+            // use WeakRef
+            fileSystemManager.setFilesCache(new WeakRefFilesCache());
+            fileSystemManager.init();
+
+            String path = "http5://www.w3schools.com/webservices/tempconvert.asmx?action=WSDL";
+            AbstractFileSystem http4FileSystem = getFile(fileSystemManager, path);

Review Comment:
   Didn't you mean http5FileSystem since we are testing Http5FileSystem?



-- 
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: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [commons-vfs] garydgregory commented on pull request #300: VFS-824 HttpFileSystem free Unused Resources lead to HttpClient Conn…

Posted by GitBox <gi...@apache.org>.
garydgregory commented on PR #300:
URL: https://github.com/apache/commons-vfs/pull/300#issuecomment-1250832322

   Hello @destroydestiny 
   Thank for your PR.
   Please see my scattered comments. 
   


-- 
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: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [commons-vfs] garydgregory commented on a diff in pull request #300: VFS-824 HttpFileSystem free Unused Resources lead to HttpClient Conn…

Posted by GitBox <gi...@apache.org>.
garydgregory commented on code in PR #300:
URL: https://github.com/apache/commons-vfs/pull/300#discussion_r976581961


##########
commons-vfs2/src/test/java/org/apache/commons/vfs2/provider/http5/Http5ProviderTestCase.java:
##########
@@ -210,4 +213,36 @@ public void testResolveFolderSlashYesRedirectOn() throws FileSystemException {
         testResolveFolderSlash(ConnectionUri + "/read-tests/", true);
     }
 
+    @Test
+    public void testHttp5FileSystemFreeUnusedResources() throws Exception {
+        try (StandardFileSystemManager fileSystemManager = new StandardFileSystemManager()) {
+            fileSystemManager.setConfiguration(StandardFileSystemManager.class.getResource("providers.xml"));
+            // use WeakRef
+            fileSystemManager.setFilesCache(new WeakRefFilesCache());
+            fileSystemManager.init();
+
+            String path = "http5://www.w3schools.com/webservices/tempconvert.asmx?action=WSDL";

Review Comment:
   Don't use a remote server. This cause the test to fail randomly and eventually fail all the time. See the other HTTP test in this class which uses an embedded HTTP server.



-- 
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: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [commons-vfs] garydgregory commented on a diff in pull request #300: VFS-824 HttpFileSystem free Unused Resources lead to HttpClient Conn…

Posted by GitBox <gi...@apache.org>.
garydgregory commented on code in PR #300:
URL: https://github.com/apache/commons-vfs/pull/300#discussion_r976573836


##########
commons-vfs2/src/test/java/org/apache/commons/vfs2/provider/http/HttpProviderTestCase.java:
##########
@@ -212,4 +215,36 @@ public void testResolveFolderSlashYesRedirectOn() throws FileSystemException {
         testResolveFolderSlash(ConnectionUri + "/read-tests/", true);
     }
 
+    @Test
+    public void testHttpFileSystemFreeUnusedResources() throws Exception {
+        try (StandardFileSystemManager fileSystemManager = new StandardFileSystemManager()) {
+            fileSystemManager.setConfiguration(StandardFileSystemManager.class.getResource("providers.xml"));
+            // use WeakRef
+            fileSystemManager.setFilesCache(new WeakRefFilesCache());
+            fileSystemManager.init();
+
+            String path = "http://www.w3schools.com/webservices/tempconvert.asmx?action=WSDL";
+            AbstractFileSystem http4FileSystem = getFile(fileSystemManager, path);

Review Comment:
   Didn't you mean httpFileSystem since we are testing HttpFileSystem?



-- 
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: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [commons-vfs] destroydestiny commented on a diff in pull request #300: VFS-824 HttpFileSystem free Unused Resources lead to HttpClient Conn…

Posted by GitBox <gi...@apache.org>.
destroydestiny commented on code in PR #300:
URL: https://github.com/apache/commons-vfs/pull/300#discussion_r974831684


##########
commons-vfs2/src/main/java/org/apache/commons/vfs2/provider/AbstractFileProvider.java:
##########
@@ -148,6 +148,21 @@ public void freeUnusedResources() {
                                       .forEach(AbstractFileSystem::closeCommunicationLink);
     }
 
+    /**
+     * get snapshot of all fileSystem
+     *
+     * @return FileSystem Array
+     */
+    public AbstractFileSystem[] getAllFileSystemSnapshot() {

Review Comment:
   Hello @garydgregory 
   Thank for your review. This is the first time I have participated in an open source project. There are many things I don't know about in terms of specifications.
   This problem was encountered in my work, when FileSystemManager.freeUnusedResources, HttpFileSystem will close the httpClient, and using the httpClient again will report an error.
   I have two immature solutions.
   1、 Close the HttpFileSystem when FreeUnusedResources, because the httpClient it holds is closed and cannot be used again. with commit 7d263eeda5c6a8b8261be22b1891ab330f9b3a06
   2、 httpclient shared ConnectionManager.  with commit 7c09877efd490903289c44e504c48034f87f08f2
   I don't know enough about Common VFS, which is definitely not the optimal solution.
   
   This method copy from  org.apache.commons.vfs2.provider.AbstractFileProvider#freeUnusedResources.  I just close FileSystem after closeCommunicationLink .
   if return a list.clone() , need to convert List<FileSystem>  to List<AbstractFileSystem> and why not create stream manually.



-- 
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: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [commons-vfs] garydgregory commented on pull request #300: VFS-824 HttpFileSystem free Unused Resources lead to HttpClient Conn…

Posted by GitBox <gi...@apache.org>.
garydgregory commented on PR #300:
URL: https://github.com/apache/commons-vfs/pull/300#issuecomment-1382453475

   It's possible one of these changes is breaking the build randomly.


-- 
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: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [commons-vfs] destroydestiny commented on pull request #300: VFS-824 HttpFileSystem free Unused Resources lead to HttpClient Conn…

Posted by GitBox <gi...@apache.org>.
destroydestiny commented on PR #300:
URL: https://github.com/apache/commons-vfs/pull/300#issuecomment-1253857732

   > Hello @destroydestiny Thank you for your updates. Please see my comments.
   
   Hi @garydgregory 
   Thank you so much for the pointers! I need to ask you how to write code rigorously.
   Cheers,


-- 
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: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [commons-vfs] garydgregory commented on a diff in pull request #300: VFS-824 HttpFileSystem free Unused Resources lead to HttpClient Conn…

Posted by GitBox <gi...@apache.org>.
garydgregory commented on code in PR #300:
URL: https://github.com/apache/commons-vfs/pull/300#discussion_r976581502


##########
commons-vfs2/src/test/java/org/apache/commons/vfs2/provider/http/HttpProviderTestCase.java:
##########
@@ -212,4 +215,36 @@ public void testResolveFolderSlashYesRedirectOn() throws FileSystemException {
         testResolveFolderSlash(ConnectionUri + "/read-tests/", true);
     }
 
+    @Test
+    public void testHttpFileSystemFreeUnusedResources() throws Exception {
+        try (StandardFileSystemManager fileSystemManager = new StandardFileSystemManager()) {
+            fileSystemManager.setConfiguration(StandardFileSystemManager.class.getResource("providers.xml"));
+            // use WeakRef
+            fileSystemManager.setFilesCache(new WeakRefFilesCache());
+            fileSystemManager.init();
+
+            String path = "http://www.w3schools.com/webservices/tempconvert.asmx?action=WSDL";

Review Comment:
   Don't use a remote server. This cause the test to fail randomly and eventually fail all the time. See the other HTTP test in this class which uses an embedded HTTP server.



##########
commons-vfs2/src/test/java/org/apache/commons/vfs2/provider/http4/Http4ProviderTestCase.java:
##########
@@ -216,4 +219,35 @@ public void testResolveFolderSlashYesRedirectOn() throws FileSystemException {
         testResolveFolderSlash(ConnectionUri + "/read-tests/", true);
     }
 
+    @Test
+    public void testHttp4FileSystemFreeUnusedResources() throws Exception {
+        try (StandardFileSystemManager fileSystemManager = new StandardFileSystemManager()) {
+            fileSystemManager.setConfiguration(StandardFileSystemManager.class.getResource("providers.xml"));
+            // use WeakRef
+            fileSystemManager.setFilesCache(new WeakRefFilesCache());
+            fileSystemManager.init();
+
+            String path = "http4://www.w3schools.com/webservices/tempconvert.asmx?action=WSDL";

Review Comment:
   Don't use a remote server. This cause the test to fail randomly and eventually fail all the time. See the other HTTP test in this class which uses an embedded HTTP server.



-- 
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: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [commons-vfs] garydgregory commented on pull request #300: VFS-824 HttpFileSystem free Unused Resources lead to HttpClient Conn…

Posted by GitBox <gi...@apache.org>.
garydgregory commented on PR #300:
URL: https://github.com/apache/commons-vfs/pull/300#issuecomment-1250825897

   https://issues.apache.org/jira/browse/VFS-824


-- 
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: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [commons-vfs] garydgregory commented on pull request #300: VFS-824 HttpFileSystem free Unused Resources lead to HttpClient Conn…

Posted by GitBox <gi...@apache.org>.
garydgregory commented on PR #300:
URL: https://github.com/apache/commons-vfs/pull/300#issuecomment-1250831287

   How about the hhtp5 file provider?


-- 
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: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [commons-vfs] destroydestiny commented on pull request #300: VFS-824 HttpFileSystem free Unused Resources lead to HttpClient Conn…

Posted by GitBox <gi...@apache.org>.
destroydestiny commented on PR #300:
URL: https://github.com/apache/commons-vfs/pull/300#issuecomment-1250933487

   > How about the http5 file provider?
   
   Http, http4, http5 provider has the same problem


-- 
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: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [commons-vfs] garydgregory commented on a diff in pull request #300: VFS-824 HttpFileSystem free Unused Resources lead to HttpClient Conn…

Posted by GitBox <gi...@apache.org>.
garydgregory commented on code in PR #300:
URL: https://github.com/apache/commons-vfs/pull/300#discussion_r976575971


##########
commons-vfs2/src/test/java/org/apache/commons/vfs2/provider/http5/Http5ProviderTestCase.java:
##########
@@ -210,4 +213,36 @@ public void testResolveFolderSlashYesRedirectOn() throws FileSystemException {
         testResolveFolderSlash(ConnectionUri + "/read-tests/", true);
     }
 
+    @Test
+    public void testHttp5FileSystemFreeUnusedResources() throws Exception {
+        try (StandardFileSystemManager fileSystemManager = new StandardFileSystemManager()) {
+            fileSystemManager.setConfiguration(StandardFileSystemManager.class.getResource("providers.xml"));
+            // use WeakRef
+            fileSystemManager.setFilesCache(new WeakRefFilesCache());
+            fileSystemManager.init();
+
+            String path = "http5://www.w3schools.com/webservices/tempconvert.asmx?action=WSDL";
+            AbstractFileSystem http4FileSystem = getFile(fileSystemManager, path);
+            http4FileSystem.isReleaseable();
+
+            while (!http4FileSystem.isReleaseable()) {

Review Comment:
   I would fail the test if a timeout is exceeded, probably with the Timeout annotation. We do not want an infinite loop.



##########
commons-vfs2/src/test/java/org/apache/commons/vfs2/provider/http4/Http4ProviderTestCase.java:
##########
@@ -216,4 +219,35 @@ public void testResolveFolderSlashYesRedirectOn() throws FileSystemException {
         testResolveFolderSlash(ConnectionUri + "/read-tests/", true);
     }
 
+    @Test
+    public void testHttp4FileSystemFreeUnusedResources() throws Exception {
+        try (StandardFileSystemManager fileSystemManager = new StandardFileSystemManager()) {
+            fileSystemManager.setConfiguration(StandardFileSystemManager.class.getResource("providers.xml"));
+            // use WeakRef
+            fileSystemManager.setFilesCache(new WeakRefFilesCache());
+            fileSystemManager.init();
+
+            String path = "http4://www.w3schools.com/webservices/tempconvert.asmx?action=WSDL";
+            AbstractFileSystem http4FileSystem = getFile(fileSystemManager, path);
+            http4FileSystem.isReleaseable();
+
+            while (!http4FileSystem.isReleaseable()) {

Review Comment:
   I would fail the test if a timeout is exceeded, probably with the Timeout annotation. We do not want an infinite loop.



-- 
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: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [commons-vfs] garydgregory commented on a diff in pull request #300: VFS-824 HttpFileSystem free Unused Resources lead to HttpClient Conn…

Posted by GitBox <gi...@apache.org>.
garydgregory commented on code in PR #300:
URL: https://github.com/apache/commons-vfs/pull/300#discussion_r974081831


##########
commons-vfs2/src/main/java/org/apache/commons/vfs2/provider/AbstractFileProvider.java:
##########
@@ -148,6 +148,21 @@ public void freeUnusedResources() {
                                       .forEach(AbstractFileSystem::closeCommunicationLink);
     }
 
+    /**
+     * get snapshot of all fileSystem
+     *
+     * @return FileSystem Array
+     */
+    public AbstractFileSystem[] getAllFileSystemSnapshot() {

Review Comment:
   Why not simply return a list.clone() which would also avoid the need to manually create streams.



##########
commons-vfs2/src/main/java/org/apache/commons/vfs2/provider/AbstractFileProvider.java:
##########
@@ -148,6 +148,21 @@ public void freeUnusedResources() {
                                       .forEach(AbstractFileSystem::closeCommunicationLink);
     }
 
+    /**
+     * get snapshot of all fileSystem

Review Comment:
   "get" -> "Gets"



##########
commons-vfs2/src/main/java/org/apache/commons/vfs2/provider/AbstractFileProvider.java:
##########
@@ -148,6 +148,21 @@ public void freeUnusedResources() {
                                       .forEach(AbstractFileSystem::closeCommunicationLink);
     }
 
+    /**
+     * get snapshot of all fileSystem
+     *
+     * @return FileSystem Array

Review Comment:
   New public APIs should have an '\@since' tag.



-- 
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: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [commons-vfs] destroydestiny commented on a diff in pull request #300: VFS-824 HttpFileSystem free Unused Resources lead to HttpClient Conn…

Posted by GitBox <gi...@apache.org>.
destroydestiny commented on code in PR #300:
URL: https://github.com/apache/commons-vfs/pull/300#discussion_r974832009


##########
commons-vfs2/src/main/java/org/apache/commons/vfs2/provider/AbstractFileProvider.java:
##########
@@ -148,6 +148,21 @@ public void freeUnusedResources() {
                                       .forEach(AbstractFileSystem::closeCommunicationLink);
     }
 
+    /**
+     * get snapshot of all fileSystem

Review Comment:
   already fixed



##########
commons-vfs2/src/main/java/org/apache/commons/vfs2/provider/AbstractFileProvider.java:
##########
@@ -148,6 +148,21 @@ public void freeUnusedResources() {
                                       .forEach(AbstractFileSystem::closeCommunicationLink);
     }
 
+    /**
+     * get snapshot of all fileSystem
+     *
+     * @return FileSystem Array

Review Comment:
   already fixed



-- 
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: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [commons-vfs] garydgregory commented on a diff in pull request #300: VFS-824 HttpFileSystem free Unused Resources lead to HttpClient Conn…

Posted by GitBox <gi...@apache.org>.
garydgregory commented on code in PR #300:
URL: https://github.com/apache/commons-vfs/pull/300#discussion_r976572059


##########
commons-vfs2/src/main/java/org/apache/commons/vfs2/provider/http4/Http4FileProvider.java:
##########
@@ -378,4 +380,17 @@ private HttpHost getProxyHttpHost(final Http4FileSystemConfigBuilder builder,
         return null;
     }
 
+
+    /**
+     * Frees unused resources and close HttpFileSystem.

Review Comment:
   HttpFileSystem is the wrong class name here, didn't you mean Http4FileSystem?



##########
commons-vfs2/src/main/java/org/apache/commons/vfs2/provider/http5/Http5FileProvider.java:
##########
@@ -375,4 +376,18 @@ private HttpHost getProxyHttpHost(final Http5FileSystemConfigBuilder builder,
         return null;
     }
 
+
+    /**
+     * Frees unused resources and close HttpFileSystem.

Review Comment:
   HttpFileSystem is the wrong class name here, didn't you mean Http5FileSystem?



-- 
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: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [commons-vfs] garydgregory commented on pull request #300: VFS-824 HttpFileSystem free Unused Resources lead to HttpClient Conn…

Posted by GitBox <gi...@apache.org>.
garydgregory commented on PR #300:
URL: https://github.com/apache/commons-vfs/pull/300#issuecomment-1253012294

   You must have not run a local build with the default Maven goal (`mvn`):
   ```
   [ERROR] src/main/java/org/apache/commons/vfs2/provider/AbstractFileProvider.java:[19,17] (imports) AvoidStarImport: Using the '.*' form of import should be avoided - java.util.*.
   ```


-- 
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: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [commons-vfs] garydgregory commented on a diff in pull request #300: VFS-824 HttpFileSystem free Unused Resources lead to HttpClient Conn…

Posted by GitBox <gi...@apache.org>.
garydgregory commented on code in PR #300:
URL: https://github.com/apache/commons-vfs/pull/300#discussion_r976576266


##########
commons-vfs2/src/test/java/org/apache/commons/vfs2/provider/http/HttpProviderTestCase.java:
##########
@@ -212,4 +215,36 @@ public void testResolveFolderSlashYesRedirectOn() throws FileSystemException {
         testResolveFolderSlash(ConnectionUri + "/read-tests/", true);
     }
 
+    @Test
+    public void testHttpFileSystemFreeUnusedResources() throws Exception {
+        try (StandardFileSystemManager fileSystemManager = new StandardFileSystemManager()) {
+            fileSystemManager.setConfiguration(StandardFileSystemManager.class.getResource("providers.xml"));
+            // use WeakRef
+            fileSystemManager.setFilesCache(new WeakRefFilesCache());
+            fileSystemManager.init();
+
+            String path = "http://www.w3schools.com/webservices/tempconvert.asmx?action=WSDL";
+            AbstractFileSystem http4FileSystem = getFile(fileSystemManager, path);
+            http4FileSystem.isReleaseable();
+
+            while (!http4FileSystem.isReleaseable()) {

Review Comment:
   I would fail the test if a timeout is exceeded, probably with the Timeout annotation. We do not want an infinite loop.



-- 
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: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [commons-vfs] destroydestiny commented on pull request #300: VFS-824 HttpFileSystem free Unused Resources lead to HttpClient Conn…

Posted by "destroydestiny (via GitHub)" <gi...@apache.org>.
destroydestiny commented on PR #300:
URL: https://github.com/apache/commons-vfs/pull/300#issuecomment-1407968556

   > I need to look at this more closely over the weekend: I don't know why the HTTP providers have to be unique and different compared to all the others. Is the use of concepts in this PR backward? In the PR, the "free" code now also "closes" resources and that feels backward to me. I expect the "close" code to also "free" resources are part of closing, not the other way around. Any thoughts?
   
   I think this problem needs to be considered from the beginning. The root cause is that "free" code closes httpClient.
   Refer to org.apache.commons.vfs2.provider.http4.Http4FileSystem#doCloseCommunicationLink
   


-- 
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: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org