in TlsActor flush data to user only after handshake has finished (#1128)

[pjfanning]

cherry pick 1e41829 #1128

[raboof]

I'm not sure this needs backporting, it seems pretty obscure and not have security impact, but I'm not opposed either.

[mdedetrich]

Im also on the fence on this one, people may be relying on this behaviour even though its "wrong" and I am not sure I would categorize this as critical

[raboof]

Im also on the fence on this one, people may be relying on this behaviour even though its "wrong"

(that risk seems relatively low to me)

and I am not sure I would categorize this as critical

agree

[mdedetrich]

(that risk seems relatively low to me)

Oh I agree its quite low, but the whole point of the 1.0.x branch is to keep the behaviour as close as possible to original Akka 2.6.x to minimize surprises when people migrate (hence why no changes unless its security or critical)

@pjfanning wdyt ?

[pjfanning]

Ok. I'll close this. I think this could be classified as a bug because it affects TLS v1.3 users but since we are getting close to a 1.1 milestone, maybe, we can delay the fix to 1.1.

[mdedetrich]

We can always decide to backport this later if someone brings it up but its always been the intention that the 1.0.x is just a step up to the 1.1.x branch which contains all the improvements/fixes etc etc

[TjarkoG]

Hi
Will 1.1.0 be released soon? otherwise i would ask if this could be reopened and merged, since it allowes the
fix for #442 to actually work. Without it the flushToUser function resets the workaround counter on every loop before we check if we should break it.
this should be the reason we're seeing the 100% cpu bug again even with the fixing commit being merged:
#842.
comparing #443 and akka/akka@v2.6.20...v2.6.21 we can actualy see, that the removal of flushToUser was missing in the change within pekko

[pjfanning]

@TjarkoG Have you tried v1.1.0-M1? I would hope that v1.1.0 would be released in the next month or 2.
I, personally, wouldn't be against backporting the case NEED_WRAP part of this PR's changes to 1.0.x but I would need to get that approved by other Pekko team members.

[mdedetrich]

Indeed 1.1.0-M1 has been released so you should be able to use that.

I am on the fence about backporting this, @raboof have your thoughts changed on this?

[TjarkoG]

is the 1.1.0-M1 production ready?
i thought that the Milestone-releases should not be used in production.
we have thus not yet used the 1.1.0-M1 release.

[raboof]

I am on the fence about backporting this, @raboof have your thoughts changed on this?

My thoughts haven't changed: I'm not opposed to backporting, but ideally I'd rather see us put effort into releasing v1.1.0 than into backporting.

is the 1.1.0-M1 production ready? i thought that the Milestone-releases should not be used in production. we have thus not yet used the 1.1.0-M1 release.

Indeed we're not yet recommending 1.1.0-M1 for general purpose production use (https://github.com/apache/pekko/blob/main/docs/src/main/paradox/release-notes/releases-1.1.md#110-m1 - it seems we haven't published that yet?). However, if have any capacity, if you could help by running 1.1.0-M1 and confirming whether it fixes the issue for you (and generally appears to work well), that might speed up how quickly we feel confident to release 1.1.0 .

[TjarkoG]

i'm assuming that we stick with semantic versioning and 1.1.0 does not have breaking changes compared to 1.0.x?
we'll test it in some of our less critical applications and i'll give feedback if everything works as expected.
might take a little since the bug is not reproduceable and occurs randomly.

[raboof]

i'm assuming that we stick with semantic versioning and 1.1.0 does not have breaking changes compared to 1.0.x?

We've documented our compatibility rules at https://pekko.apache.org/docs/pekko/current/common/binary-compatibility-rules.html - indeed if you weren't using experimental or deprecated APIs on 1.0.x I don't think you should expect anything to break on 1.1.x. Of course testing before going in production is still advised :)

we'll test it in some of our less critical applications and i'll give feedback if everything works as expected. might take a little since the bug is not reproduceable and occurs randomly.

That'd be much appreciated!

[TjarkoG]

sadly we're currently blocked by an error within kamon instrumentation when updating to 1.1.0-M1
kamon-io/Kamon#1352
once this is fixed we'll get back