[GitHub] [commons-vfs] boris-petrov commented on pull request #166: Remove usage of `ThreadLocal` in `DefaultFileContent`

[GitBox <gi...@apache.org>]

boris-petrov commented on pull request #166:
URL: https://github.com/apache/commons-vfs/pull/166#issuecomment-796813851


   > Hi @boris-petrov
   > Thank you for PR.
   > May you please rebase on `master` to pick up the latest to make sure we are still green.
   
   Done.
   
   > Previously, at most one `OutputStream` was allowed _per thread_. With your change, at most one `OutputStream` is allowed _globally_. This might break applications.
   > 
   > It's about this piece of code:
   > 
   > https://github.com/apache/commons-vfs/blob/a19631a852cdef62533566c158788f1bd221fa50/commons-vfs2/src/main/java/org/apache/commons/vfs2/provider/DefaultFileContent.java#L532-L534
   
   Will check this later/tomorrow.
   
   > Hi @boris-petrov
   > 
   > > This also fixes inability to close a stream from a different thread
   > 
   > That's one you can write a test to prove the fix actually works.
   
   Will do.
   
   > One more bug: TOCTOU bug in all functions which set `this.fileContentThreadData = null` after checking it's empty. Between the check and the assignment, another thread may have created a new stream.
   
   That's pretty much the issue I explained in the original post. As I said there, it was always there and could/should be fixed by another PR.


----------------------------------------------------------------
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:
users@infra.apache.org