Yes, people will write bad coders which is why this is there. No, I don't think swallowing one close is what we want.
In the case where you wants to pass an input/output stream to a library that incorrectly assumes ownership, the input/output stream should be wrapped right before the call to the library with a filter input/output stream that swallows the close and not propagate ignoring this bad behavior elsewhere. On Wed, Jan 31, 2018 at 12:04 PM, Romain Manni-Bucau <[email protected]> wrote: > Hmm, here we are the ones owning the call since it is in a coder, no? Do > we assume people will badly implement coders? In this particular case we > can assume close() will be called by a framework I think. > What about swallowing one close() and fail on the second? > > > Romain Manni-Bucau > @rmannibucau <https://twitter.com/rmannibucau> | Blog > <https://rmannibucau.metawerx.net/> | Old Blog > <http://rmannibucau.wordpress.com> | Github > <https://github.com/rmannibucau> | LinkedIn > <https://www.linkedin.com/in/rmannibucau> > > 2018-01-31 20:59 GMT+01:00 Lukasz Cwik <[email protected]>: > >> Because people write code like: >> myMethod(InputStream in) { >> InputStream child = new InputStream(in); >> child.close(); >> } >> >> InputStream in = new FileInputStream(... path ...); >> myMethod(in); >> myMethod(in); >> >> An exception will be thrown when the second myMethod call occurs. >> >> Unfortunately not everyone wraps their calls to a coder with an >> UnownedInputStream or a filter input stream which drops close() calls is >> why its a problem and in the few places it is done it is used to prevent >> bugs from creeping in. >> >> >> >> On Tue, Jan 30, 2018 at 11:29 AM, Romain Manni-Bucau < >> [email protected]> wrote: >> >>> I get the issue but I don't get the last part. Concretely we can support >>> any lib by just removing the exception in the close, no? What would be the >>> issue? No additional wrapper, no lib integration issue. >>> >>> >>> Romain Manni-Bucau >>> @rmannibucau <https://twitter.com/rmannibucau> | Blog >>> <https://rmannibucau.metawerx.net/> | Old Blog >>> <http://rmannibucau.wordpress.com> | Github >>> <https://github.com/rmannibucau> | LinkedIn >>> <https://www.linkedin.com/in/rmannibucau> >>> >>> 2018-01-30 19:29 GMT+01:00 Lukasz Cwik <[email protected]>: >>> >>>> Its common in the code base that input and output streams are passed >>>> around and the caller is responsible for closing it, not the callee. The >>>> UnownedInputStream is to guard against libraries that are poorly behaved >>>> and assume they get ownership of the stream when it is given to them. >>>> >>>> In the code: >>>> myMethod(InputStream in) { >>>> InputStream child = new InputStream(in); >>>> child.close(); >>>> } >>>> >>>> InputStream in = ... >>>> myMethod(in); >>>> myMethod(in); >>>> When should "in" be closed? >>>> >>>> To get around this issue, create a filter input/output stream that >>>> ignores close calls like on the JAXB coder: >>>> https://github.com/apache/beam/blob/master/sdks/java/io/xml/ >>>> src/main/java/org/apache/beam/sdk/io/xml/JAXBCoder.java#L181 >>>> >>>> We can instead swap around this pattern so that the caller guards >>>> against callees closing by wrapping with a filter input/output stream but >>>> this costs an additional method call for each input/output stream call. >>>> >>>> >>>> On Tue, Jan 30, 2018 at 10:04 AM, Romain Manni-Bucau < >>>> [email protected]> wrote: >>>> >>>>> Hi guys, >>>>> >>>>> All is in the subject ;) >>>>> >>>>> Rational is to support any I/O library and not fail when the close is >>>>> encapsulated. >>>>> >>>>> Any blocker to swallow this close call? >>>>> >>>>> Romain Manni-Bucau >>>>> @rmannibucau <https://twitter.com/rmannibucau> | Blog >>>>> <https://rmannibucau.metawerx.net/> | Old Blog >>>>> <http://rmannibucau.wordpress.com> | Github >>>>> <https://github.com/rmannibucau> | LinkedIn >>>>> <https://www.linkedin.com/in/rmannibucau> >>>>> >>>> >>>> >>> >> >
