fix(rt): enforce only once shutdown logic for crt engine connections #497
Conversation
fixes: #413 Fixes the segfault that can happen when an exception is handled twice leading to a connection being closed after it has been free'd. This change refactors the handling of the connection close logic to be handled in a single place regardless of why the connection is being closed.
|
A new generated diff is ready to view: __generated-main...__generated-fix-413 |
|
A new generated diff is ready to view: __generated-main...__generated-fix-413 |
| val forceClose = !streamCompleted | ||
|
|
||
| if (forceClose) { | ||
| logger.trace { "stream did not complete before job, forcing connection shutdown! handler=$this; conn=$conn; stream=$crtStream" } |
There was a problem hiding this comment.
comment
Perhaps this should be a higher level message, like warn. From past experience it is often useful to know when partial results where received.
There was a problem hiding this comment.
This doesn't necessarily mean partial results though. Technically it just means the coroutine job completed before consuming the entire response which could happen because of an exception or simply because the consumer doesn't read the entire body.
If we actually had a stream cancellation API then we wouldn't need to force close the connection which would make this less heavy handed.
I don't think I'd make it warn because in the case of an exception they'll be forced to handle it anyway and the exception is your signal. In the case of explicitly not consuming the entire body it's not a warning since the consumer is choosing it. Perhaps debug would be better than trace though?
|
Kudos, SonarCloud Quality Gate passed!
|
|
A new generated diff is ready to view: __generated-main...__generated-fix-413 |
Issue #
fixes #413
Description of changes
Fixes the segfault that can happen when an exception is handled twice leading to a connection being closed after it has been free'd. This change refactors the handling of the connection close logic to be handled in a single place regardless of why the connection is being closed.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.