clarify whether null is a valid element

[rkuhn]

• Java’s Stream supports it
• most concurrent data structures do not support it

[viktorklang]

Given that there is no provision in the spec prohibiting the use of null, I think the current status is that it is prohibited. @reactive-streams/contributors Thoughts?

[ktoso]

I believe it is currently not prohibited to use null as an element.
TCK wise we never check for values, other than ' the same element' in one or two tests with multiple subscribers.

So this clarification needs no tck updates either way (as forbidding null would be a spec violation rule, so I'd assume we do not force subscribers to throw if someone signals a null value).

// edited a bit to make it clearer, initial wording I used was confusing - sorry

[drewhk]

For implementors those nulls will be hellish, most of the concurrent queues dislike it.

[DougLea]

I don't see why it needs to be specified. The only method this could impact is onNext. Some (hopefully most) publishers will promise never to call onNext(null) because it would make no sense, and some (hopefully most) subscribers would treat them as indications that there is nothing-there, and either skip them or treat them as errors.

[viktorklang]

@DougLea I suspect the problematic case will be the combination of null-publishing-publisher with non-null-receiving-subscriber.
In any case, it is completely possible to use a null-token to transport null safely in a CLQ for example.

[drewhk]

yes. but those null-tokens will be custom per library?

[drewhk]

or you mean that the users of a CLQ just do it internally

[viktorklang]

@drewhk I mean that since it is the CLQ of the Subscriber, any such handling is purely local.

[drewhk]

Ah forget it, I am too tired to think, no problem there.

[DougLea]

To clarify: Banning nulls is a great policy because it avoids unsolvable issues about how to interpret them: In most concurrent settings, sending "nothing" makes no sense, so it is usually banned, which also simplifies APIs. But if nulls are allowed to be sent here, it ought to be legal for any receiving stage (including relays) to skip them. (Thus they never enter queues etc.)

[viktorklang]

I'm all for banning null from onNext, would be great to get more opinions from @reactive-streams/contributors on this topic :)

[benjchristensen]

In RxJava we have allowed it and use a NULL_SENTINEL whenever we are going via a queue. I don't have strong opinion either direction on allowing or banning it, just sharing that we did end up making the effort to support it, and we have had users over time who wanted the support (and have filed bugs when we didn't).

[danarmak]

FWIW, future-streams supports nulls (without any special treatment); I have no reason to disallow them.

If a user doesn't realize nulls are disallowed and tries using them, maybe the spec should require immediate runtime failure on calling onNext(null). Otherwise the resulting error may be hard to understand for someone who isn't familiar with the implementation used. CLQ.add(null) throws a NullPointerException, but in theory an implementation could rely on the prohibition of nulls in a way that makes sending nulls a race condition or some other undefined, hard to debug behavior.

@DougLea why are you suggesting subscribers skip nulls? In most cases I would expect an error instead of silently ignoring input. If I write a Processor that maps elements, and it happens to be implemented using CLQ inside my streams library (not manually by me as a user), and it encounters a null, then I would want it to signal an error, not silently drop input.

[DougLea]

@danarmak I just sent you 42 empty TCP packets as an answer.

Again, the main problem here is that there is no single right way to process nulls, so users who care about quality won't use them. (The in-progress j.u.c SubmissionPublisher does not allow them to be sent.) But that's not necessarily a reason to ban them outright.

[danarmak]

@DougLea certainly, sometimes it's permissible or even expected to drop certain messages. But unless the subscriber understands the semantics of null stream elements, it shouldn't drop them. Just like a j.u.Collection may not support nulls, but then calling add(null) should throw a NullPointerException, and not 'drop' the element and return successfully.

Also, "empty" elements are often represented by something other than null (e.g. ByteString.empty), so the logic of what to drop needs to be explicitly added to the stream. IOW, I wouldn't want to rely on implicitly dropping nulls unless the spec outright requires it.

I think we should do one of:

1. Require implementations to support nulls. This is hard for some of them, so we don't want to.
2. Forbid implementations to support nulls, and require calls to onNext(null) results in an error (cancelling the subscription). This has the benefit of consistency, failing early, and making it obvious what caused the error. Every implementation has to remember to add a trivial check to onNext, and users who want to use nulls (because they naturally appear in their model) need to replace them with some other value.
3. Allow implementations to do whatever they want, as long as they document it, just as different collections may or may not support nulls.

Do you think some other option involving dropping is better?

[DougLea]

My attitude is colored by a long history of dealing with this. I'd prefer to ban the use of null to mean anything other than "there is nothing here". But I settle for doing this only in components that I develop, and let people adapt to it using sentinels etc if they need to. Which amounts to your option (3).

[viktorklang]

The problem here (that I see) is that if onNext-ing null is OK to be silently dropped, it needs to be specced so that demand counting doesn't go awry.

I'm fine with either A) Explicitly mandate that null is fine to send or B) Explicitly mandate that null yields an NPE from onSubscribe(null), onNext(null) or onError(null).

@reactive-streams/contributors Thoughts? :)

[drewhk]

I vote for forbidding null.

[benjchristensen]

I don't have a strong opinion on this so I'm fine either way. Existing stream libraries supporting nulls will have to start throwing an NPE when bridging into Reactive Streams if we go that route, but I'm fine with that.

[akarnokd]

Subscribe(null) has nowhere to go (can't throw, can't call null.onError). For the other methods, I think the cost of enforcing non-nullness is greater than using null-sentinels. The former need to be put on every subscriber frontend at every stage (increasing method bytecode length as well), whereas the latter need to be put in front and back of helper data structures (i.e., queues) only.

Generally, the issue I see with forced onError based error reporting (since the 7 API methods mustn't throw) is that Subscribers need to be defensively serializing because rule violations happening on any thread has to be reported spec conformant.

[viktorklang]

@akarnokd Indeed, the behavior of Publisher.subscribe(null) should be outlined in the spec. Given that it is on the "outside" of the interactions (i.e. the first interaction) I'd think an IAE or NPE would be permissible. @reactive-streams/contributors, thoughts?

[DougLea]

@viktorklang Same answer as onNext(null) (also for onSubscribe(null), onError(null)): Either say that behavior is undefined and implementation-dependent (thus allowing exceptions), or mandate that they cannot be null (thus requiring exceptions). Either approach conveys the message that reliable programs do not use nulls as sentinels.

[viktorklang]

@DougLea having a ban on null would be nicely symmetric for subscribe, onSubscribe, onNext and onError.

[viktorklang]

@reactive-streams/contributors Should we add a Rule requiring all signals to throw an NPE or (IAE?) if null is passed as a parameter?

Ie. illegal:

subscribe(null)
onSubscribe(null)
onNext(null)
onError(null)

(We should then update the TCK accordingly)

[viktorklang]

I'll open a PR that can be voted upon.