On Mon, 7 Oct 2024 16:06:08 GMT, Chen Liang <li...@openjdk.org> wrote:
>> This is correct, but then we would keep the reference to the char sequence >> "forever", without any good reason. This content could be *huge* (and often >> it is, as `CharSequence` most often is a `StringBuilder` just because the >> application *wants* to create huge content, and `String`-concatenation would >> be too costly with *huge* content). In fact, I see this scenario as the top >> use case for this new API. >> >> Having said that, I would really like to have this field non-final to unlink >> the reference to the source as soon as possible, implicitly allowing GC to >> free this unused memory. > >> keep the reference to the char sequence "forever" > > Note that this only happens if the `Reader` instance is kept forever; but a > `Reader` is supposed to be used and closed and discarded instead of being > held indefinitely after closing. If users are bad enough, they can never call > `close` and the non-final `cs` is still leaked. > > The right way of GC freeing is to let the GC free the reader and the char > sequence together, instead of allowing to free when a client keeps this > reader instance erroneously after closing. I really do see you point, nevertheless I think we should be kind to "bad" users, too. In the end, there is *no* specification which explicitly tells "bad" users that readers are support to be short-living (despite it being rather obvious), while `close` explicitly mentions freeing *all* resources, so it is rather unexpected to keep something past `close`, even for short term. NB: `StringReader` applies the very same behavior, too (using `s` and `str` variables), that's where I copied the code from originally. It is not a big problem for us to have two variables, neither that the name of the variable is `source` (in fact, I do like `source` even more than `cs`, but this is just my personal preference). To sum up, I really dislike the idea to keep the reference for any longer than essentially needed. If it's just about the name of the variable, I could simply switch them if you prefer that. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/21371#discussion_r1790631189
