Using InputStream as :ssl-context :trust-store is buggy about aleph HOT 6 OPEN

I don't recall ever using an InputStream, but as you can see here, Aleph doesn't do much with it by default.

It's interesting that they fail "in quick succession". Maybe there's some delayed init that's not triggered until needed, and then if two conns both try to setup the sslcontext/trust store, they end up racing on the InputStream, and one or both fail.

It looks like that happens on the client-side. It makes a new client context for each conn, which is necessary in case the context is actually just a map of options. And if you make a call slowly, the same conn will get reused, so it's not an issue there. But too fast, and it'll spawn multiple conns, corresponding multiple sslcontexts, and try to read from an exhausted stream or in the middle of an earlier conn.

Solution is to either (1) force the stream into another format ASAP, or (2) disallow streams.

from aleph.

@KingMob My reading of the code agrees with your analysis 👍 I'll come up with a test case to reproduce it.

It looks like that happens on the client-side. It makes a new client context for each conn, which is necessary in case the context is actually just a map of options

I think this might actually not be necessary: It should be possible to lift the construction of the context to the level of the pool instead which would solve this bug as well as reduce allocation. Will give this a try!

from aleph.

Yeah, even when SslContext construction is idempotent, why do it multiple times?

from aleph.

Yeah, even when SslContext construction is idempotent, why do it multiple times?

If these instances are not thread-safe, that could be a reason. But since they are immutable, I suppose they also should be thread safe. Furthermore, I think the netty SslContext instances are based on the JDK SslContext implementations and if these weren't thread-safe it would be a widespread common problem (even though I find it hard to find explicit documentation about this).

from aleph.

@bitti Good point. Though I've never really considered, should thread safety be implicit in the definition of "idempotent"? I assumed so, but we programmers are much looser about the definition than mathematicians.

from aleph.

@bitti Good point. Though I've never really considered, should thread safety be implicit in the definition of "idempotent"? I assumed so, but we programmers are much looser about the definition than mathematicians.

I think even in the mathematical sense, you can't 'define' it like that, since thread-unsafety implies undefined behavior. So no, neither idempotency nor immutability implies thread-safety.

But I think in this case we can safely assume the JDK/netty implementations are thread-safe since otherwise it would, it make it more difficult to share connections (at least that's what I gather from the SO discussions). I gather the reason why the JDK docs don't explicitly state this is because they can't make a guarantee for the millions of potential SslContext implementations out there.

from aleph.