Expanding the horizon beyond nullity: Far-future, or is there a list?

75 views
Skip to first unread message

Reinier Zwitserloot

unread,
Sep 27, 2023, 8:13:11 AM9/27/23
to jspecify-discuss
There are a bunch of annotations that fit the vein of `@Nullable` that java simply doesn't have - things that have _all_ of these properties:

* They are useful from a 'how do I use this API?' perspective; javadoc should be mentioning it at the very least.
* Useful static analysis can be performed that isn't going to work nearly as well without it.
* It's already commonly -used- in java, both in the core libs and the wider community. Though, not always all that well documented and static analysis tools currently just flat out get it wrong from time to time, or don't analyse it, because there are no annotations.

Some are well known (I'm sure you're all thinking of something like `@SideEffectFree` or `com.google.errorprone.annotations.@CanIgnoreReturnValue`).

There's a new use case I ran across. I'm not personally familiar with any framework / annotation library that has an annotation for it:

`@ResourceResponsibilityTransfer` (the name needs work). `System.out` is a resource that shouldn't be closed - you didn't make it, when you 'read' that, it doesn't imply you are now responsible for that. In fact, quite the opposite - it's a bug if you do close it, generally. In contrast, `new FileOutputStream()` very much does imply the code that does that needs to either [A] safe-close that, or [B] return it to the caller _and_ transfer responsibility to it. `Files.newOutputStream` - that would transfer the responsibility. So does `socket.getInputStream()`. The naming is not consistent (`get`, if anything, implies that no such heavy handed things like a transfer of responsibilities has occurred, but, it has).

Automated tools get it wrong - complaining e.g. about having to close `new Scanner(System.in)` when that is entirely incorrect (as that would close `System.in` - not good).

There's even a category of optional closables: a ResultSet - eh, if you don't try-with guard that, it really doesn't matter, as closing the connection will take care of it. But you can. Trickier, but, if the code analysis tool can ensure that the `InputStream` you have is in fact a `ByteArrayInputStream`, you can close it, but you don't have to.

An annotation would be a nice start, and java's own APIs needs some external annotations too.

I was thinking about projects that should pick this up, and the mission statement of JSpecify does fit rather well.

Is there a list of 'eventually we plan to at least investigate these annotations', should I send a PR with one, or, is that more of a 'far future' thing, and the focus for now is entirely on nullity?

I'm guessing the focus lies with nullity now and that's entirely understandable. Just in case 'start listing future stuff JSpecify should try to cover' is within scope at this point, I thought I'd check in :)

Manu Sridharan

unread,
Sep 27, 2023, 11:28:55 AM9/27/23
to jspecify-discuss, Reinier Zwitserloot
Dear Reiner,

For your use case, you may want to look at the annotations defined by the Resource Leak Checker (RLC) that we developed as part of the Checker Framework:


We have an annotation @Owning (inspired by Rust-style ownership, but more lightweight) that I think is close to your @ResourceResponsibilityTransfer (and also @NotOwning).  The RLC checks must-call obligations specified with @MustCall annotations, and one can write @MustCall({}) to override the default obligation for a type (e.g., on System.out).  We also use @MustCall({}) for ByteArrayInputStream and the like.  And, we have a @MustCallAlias annotation to indicate that a wrapper type should have the same obligation as the type it is wrapping; that handles cases like `new Scanner(System.in)`.  We’d be happy to help you use the checker or to improve it with your feedback.

As to standardizing these types of annotations, I think JSpecify is pretty focused on nullity for now, but there is a bit of an eye on the future :-)  I think possibly tracking this idea as an issue at https://github.com/jspecify/jspecify/issues might be the best way to go at this point?  Perhaps others can comment better on how best to track this.

Best,
Manu


--
You received this message because you are subscribed to the Google Groups "jspecify-discuss" group.
To unsubscribe from this group and stop receiving emails from it, send an email to jspecify-discu...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/jspecify-discuss/d3ac586a-ad61-4b1b-be96-5da91e8018f2n%40googlegroups.com.

Kevin Bourrillion

unread,
Sep 27, 2023, 2:31:16 PM9/27/23
to Reinier Zwitserloot, jspecify-discuss
On Wed, Sep 27, 2023 at 5:13 AM Reinier Zwitserloot <rein...@gmail.com> wrote:

There are a bunch of annotations that fit the vein of `@Nullable` that java simply doesn't have - things that have _all_ of these properties:

* They are useful from a 'how do I use this API?' perspective; javadoc should be mentioning it at the very least.
* Useful static analysis can be performed that isn't going to work nearly as well without it.
* It's already commonly -used- in java, both in the core libs and the wider community. Though, not always all that well documented and static analysis tools currently just flat out get it wrong from time to time, or don't analyse it, because there are no annotations.

Wow. I don't have to give my "we only ever want to have annotations that meet these 3 criteria" speech! This is basically exactly how I've seen the boundaries of our scope. The annotations that fit these criteria tend to "feel almost like a language feature". And I'm determined to hold that line, to keep our library small and flat.

Here's my personal list of candidates, in a rough approximate order of important/soon to just-spitballing. That's not an official by-consensus plan, just domains we happen to be addressing with some success internally to Google.

That page makes more sense in a universe where a lot of energy is pouring into JSpecify from the partner orgs and the wider community. That waveform hasn't collapsed yet -- er, I don't know if that will prove to be the case.


Some are well known (I'm sure you're all thinking of something like `@SideEffectFree` or `com.google.errorprone.annotations.@CanIgnoreReturnValue`).

Yes! At Google we recently got "must use return value" enabled as the default behavior for all Java code, and learned a ton from that process, and standardization would be great. The overall mess of "policy regarding state" annotations is intricate and I'd hope to get a reasonable picture of the whole space before actually starting to design a part of it. I've put some time into that as well, but not shared anything yet.


There's a new use case I ran across. I'm not personally familiar with any framework / annotation library that has an annotation for it:

`@ResourceResponsibilityTransfer` (the name needs work).

I agree that this is another great example. The use case is methods returning types like Stream and CharSequence, where no matter how much we know about their different subtypes' policies, the abstract throws that info out anyway. We have to preserve it somehow. The wiki page I linked to only mentions `@MustBeClosed` but yeah, I suppose perhaps we have at least three states.

There's a whole other kind of responsibility-transfer as well. When you call `synchronizedList(myList)` you relinquish your rights to the reference you pass. That reference shouldn't be used again, nor leak from the current context (before or after the call). In the other direction, "method promises to relinquish the returned instance". These seem esoteric, and if they are, we should ignore them. If they're common yet don't tend to catch many bugs in practice, same. I seem to remember convincing myself that this kind of thing happens a lot, but :shrug: on the second part.


I'm guessing the focus lies with nullity now and that's entirely understandable. Just in case 'start listing future stuff JSpecify should try to cover' is within scope at this point, I thought I'd check in :)

We just had to pick the hardest area of them all to tackle first, and now it's been years, and it's not clear how much appetite people will have for "let's keep going to do more things". But I can at least say that it's been my explicit aspiration from the start (I wouldn't have gotten involved otherwise), and I haven't heard anyone explicitly argue the other side (we should stop at nullness).

Please keep your deep thoughts coming, this is great.


--
Kevin Bourrillion | Java/Kotlin Ecosystem Team | Google, Inc. | kev...@google.com

Reinier Zwitserloot

unread,
Sep 27, 2023, 10:37:31 PM9/27/23
to jspecify-discuss
> These seem esoteric, and if they are, we should ignore them.

Hmm, not esoteric enough, I think. It comes up a lot in those filter contexts - once you wrap an `OutputStream` into a `BufferedOutputStream`, it gets very tricky to go back to the original ever again. Rather unfortunately, not every filterstream transfers close. So I'm not sure it's actually wise to treat `new BufferedOutputStream(x)` as: That means you have fully relinquished all control over x and should never use/pass it again. I've filed a bug at OpenJDK and they have, as I unfortunately have grown to expect from them, ignored it (perhaps such thoughts are a bit too esoteric for the bug tracker, perhaps). Specifically, this is effectively safe:

try (var x = new BufferedOutputStream(Files.newOutputStream(path)) {}

But this is not:

try (var x = new ZipOutputStream(Files.newOutputStream(path)) {}

The BOS infrastructure uses try/finally properly to ensure that `bos.close()` _necessarily_ at least tries to close the underlying stream regardless of any other problems that occur. But ZipOutputStream doesn't. If writing the zip header (which comes at the end, `.close()` does that) fails, the underlying stream won't be closed.

Still, I'm pretty sure that objectively speaking the correct approach is to state that __all__ subtypes of FilterStreams should necessarily promise to close underlying safely (the compiler can't enforce that, but then, it can't enforce the rule that if `a.equals(b)` that `a.hashCode() == b.hashCode()` either. But it needs to be documented), and then to apply a relinquish protocol. Because closing the underlying outputstream _before_ closing the BufferedOutputStream that wraps it is similarly disastrous, so in spirit that relinquish idea applies here, it'd be a nice addition.

I hadn't thought of this notion that 'responsibility transfer' goes a lot further than I/O resource objects. As Manu Sridharan mentioned, rust is a bit of an inspiration. Its compile-time checking of failures in responsibility transfer, not to mention building an entire memory alloc/free system on top of it, is rather awesome.

> [the link to the long term roadmap]

Duh, I should have checked the wiki. Thanks for the link :) Glad to see it's on there, and in the second position no less.

Reinier Zwitserloot

unread,
Sep 27, 2023, 10:41:13 PM9/27/23
to jspecify-discuss
Hi Manu,

Of course checker framework thought of this. Thanks for the pointer :) - Given that it follows in the footsteps of rust (keeping track of which ref gets to decree itself as 'responsible for it'), the same system can be used for Kevin's notion that responsibility goes further than I/O resource objects. A negative constraint would be interesting (if any code path _may_ have passed an object as parameter to a method that says: If you do that, you have therefore relinquished that object and must not interact with it again - then any further interaction with it should be flagged as problematic code) - the analysis that CF does to determine responsibility for closing resources feels like it's 90% of the way there already.

Kevin Bourrillion

unread,
Oct 2, 2023, 11:51:23 AM10/2/23
to Reinier Zwitserloot, jspecify-discuss
On Wed, Sep 27, 2023 at 7:37 PM Reinier Zwitserloot <rein...@gmail.com> wrote:

> These seem esoteric, and if they are, we should ignore them.

Rather unfortunately, not every filterstream transfers close. So I'm not sure it's actually wise to treat `new BufferedOutputStream(x)` as: That means you have fully relinquished all control over x and should never use/pass it again.

Agreed, if you don't know close is delegated, this idea doesn't apply. Very sad indeed that something like this is unspecified.


Still, I'm pretty sure that objectively speaking the correct approach is to state that __all__ subtypes of FilterStreams should necessarily promise to close underlying safely (the compiler can't enforce that, but then, it can't enforce the rule that if `a.equals(b)` that `a.hashCode() == b.hashCode()` either. But it needs to be documented),

Yes. My mentality is:

* If you can prevent it, do
* If you can't prevent it, at least make it clear which code is to blame
 

> [the link to the long term roadmap]

Duh, I should have checked the wiki.

Don't worry, if you had you'd have been the first. :-)

Anyway... we will get over the nullness hump somehow, and then our big challenge is how to turn JSpecify from a nullness thing into the overall static analysis thing we need. And that "how" is more about energy/investment/organizational-willpower than any technical question. I'd be glad for your thoughts on what might help that along, now or later.



On Wednesday, September 27, 2023 at 8:31:16 PM UTC+2 kev...@google.com wrote:
On Wed, Sep 27, 2023 at 5:13 AM Reinier Zwitserloot <rein...@gmail.com> wrote:

There are a bunch of annotations that fit the vein of `@Nullable` that java simply doesn't have - things that have _all_ of these properties:

* They are useful from a 'how do I use this API?' perspective; javadoc should be mentioning it at the very least.
* Useful static analysis can be performed that isn't going to work nearly as well without it.
* It's already commonly -used- in java, both in the core libs and the wider community. Though, not always all that well documented and static analysis tools currently just flat out get it wrong from time to time, or don't analyse it, because there are no annotations.

Wow. I don't have to give my "we only ever want to have annotations that meet these 3 criteria" speech! This is basically exactly how I've seen the boundaries of our scope. The annotations that fit these criteria tend to "feel almost like a language feature". And I'm determined to hold that line, to keep our library small and flat.

Here's my personal list of candidates, in a rough approximate order of important/soon to just-spitballing. That's not an official by-consensus plan, just domains we happen to be addressing with some success internally to Google.

That page makes more sense in a universe where a lot of energy is pouring into JSpecify from the partner orgs and the wider community. That waveform hasn't collapsed yet -- er, I don't know if that will prove to be the case.


Some are well known (I'm sure you're all thinking of something like `@SideEffectFree` or `com.google.errorprone.annotations.@CanIgnoreReturnValue`).

Yes! At Google we recently got "must use return value" enabled as the default behavior for all Java code, and learned a ton from that process, and standardization would be great. The overall mess of "policy regarding state" annotations is intricate and I'd hope to get a reasonable picture of the whole space before actually starting to design a part of it. I've put some time into that as well, but not shared anything yet.


There's a new use case I ran across. I'm not personally familiar with any framework / annotation library that has an annotation for it:

`@ResourceResponsibilityTransfer` (the name needs work).

I agree that this is another great example. The use case is methods returning types like Stream and CharSequence, where no matter how much we know about their different subtypes' policies, the abstract throws that info out anyway. We have to preserve it somehow. The wiki page I linked to only mentions `@MustBeClosed` but yeah, I suppose perhaps we have at least three states.

There's a whole other kind of responsibility-transfer as well. When you call `synchronizedList(myList)` you relinquish your rights to the reference you pass. That reference shouldn't be used again, nor leak from the current context (before or after the call). In the other direction, "method promises to relinquish the returned instance". These seem esoteric, and if they are, we should ignore them. If they're common yet don't tend to catch many bugs in practice, same. I seem to remember convincing myself that this kind of thing happens a lot, but :shrug: on the second part.


I'm guessing the focus lies with nullity now and that's entirely understandable. Just in case 'start listing future stuff JSpecify should try to cover' is within scope at this point, I thought I'd check in :)

We just had to pick the hardest area of them all to tackle first, and now it's been years, and it's not clear how much appetite people will have for "let's keep going to do more things". But I can at least say that it's been my explicit aspiration from the start (I wouldn't have gotten involved otherwise), and I haven't heard anyone explicitly argue the other side (we should stop at nullness).

Please keep your deep thoughts coming, this is great.


--
Kevin Bourrillion | Java/Kotlin Ecosystem Team | Google, Inc. | kev...@google.com

--
You received this message because you are subscribed to the Google Groups "jspecify-discuss" group.
To unsubscribe from this group and stop receiving emails from it, send an email to jspecify-discu...@googlegroups.com.
Reply all
Reply to author
Forward
0 new messages