MaxKellermann commented on pull request #166:
URL: https://github.com/apache/commons-vfs/pull/166#issuecomment-797545144


   > Ah, I see now what the problem is... `getContent` on a `FileObject` is 
called from multiple threads (which get the same `DefaultFileContent` object) 
and then one of the threads closes the `DefaultFileContent` which will close 
all streams, including the ones for the other threads...
   
   D'oh, so that was another reason why a ThreadLocal was used - `close()` is 
supposed to close only the streams by the current thread, even though the 
documentation says it shall close *all* streams:
   
https://github.com/apache/commons-vfs/blob/90e436e82831d25c9a9a8019f7099422cf6f5485/commons-vfs2/src/main/java/org/apache/commons/vfs2/provider/DefaultFileContent.java#L434
   The new behavior is correct according to the API documentation, the old 
behavior was not correct (at least it did not do what the API documentation 
said it would do).
   
   The whole idea of this `close()` method is bad. It's not a well designed 
API. But it's there, and must not be changed, because applications may depend 
on it.
   
   > Perhaps we could not close the streams when the `DefaultFileContent` is 
closed but rather when the `FileObject` is closed?
   
   That would be an API change as well. No matter how bad the API is, it must 
be stable. (Unless somebody decides to raise this library's major version and 
break the API.)


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to