boris-petrov edited a comment on pull request #166: URL: https://github.com/apache/commons-vfs/pull/166#issuecomment-796858400
> Now `close()` is completely `synchronized`, which may hold the lock for a long time while the underlying I/O code is called. Which is fine as anyway no other methods should be called on the `DefaultFileContent` instance any more. But there is a problem with what I mentioned above: > Ah, no, this still doesn't work. In `DefaultFileContent.close` the `close` methods of the streams will call `streamClosed` in a synchronized context which is unwanted... Which I have no idea how to fix... > Also be careful to document what must be locked by the caller. It is dangerous that `getFileContentThreadData()` is not `synchronized` anymore, but you didn't document that synchronization is the caller's responsibility. In cases like that, I'd leave the `synchronized` in - this only adds negligible overhead, but is safe-by-default. Yes, I was also thinking of adding a comment. I think it's better because making it `synchronized` won't fix the TOCTOU issues of users who call the method unsychronized. ---------------------------------------------------------------- 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]
