Le 1 févr. 2018 03:10, "Lukasz Cwik" <lc...@google.com> a écrit :
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. Hmm, Elsewhere is nowhere else here since it wouldnt have any side effect except not enforcing another layer and making smoothly work for most mappers. Anyway I can live with it but I'm a bit sad it closes the door to the easyness to write extensions. On Wed, Jan 31, 2018 at 12:04 PM, Romain Manni-Bucau <rmannibu...@gmail.com> 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 <lc...@google.com>: > >> 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 < >> rmannibu...@gmail.com> 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 <lc...@google.com>: >>> >>>> 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 < >>>> rmannibu...@gmail.com> 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> >>>>> >>>> >>>> >>> >> >
