Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Release 2.3.5 #3771

Merged
merged 9 commits into from
Oct 4, 2023
Merged

Release 2.3.5 #3771

merged 9 commits into from
Oct 4, 2023

Conversation

e5l
Copy link
Member

@e5l e5l commented Oct 2, 2023

No description provided.

Sebmaster and others added 6 commits October 2, 2023 13:07
When using the Apache5 engine, total concurrent requests to a single
route were limited to 5 requests. This is due to the code which tried to
increase the concurrency to the ktor-standard 1000 concurrent having a
typo and setting the total max connections twice and missing the max
connections
completely.

(cherry picked from commit fa5cba1)
* Avoid crashing when TLS TCP socket is closed

When using a plain TCP socket with TLS, two worker jobs are spawned:

- cio-tls-input-loop
- cio-tls-output-loop

These worker jobs live in their own scope, and exceptions they throw can't
easily be caught by the caller: the only option is to install a
`CoroutineExceptionHandler` for the whole TLS socket, which is quite hacky
and may hide bugs.

When the TCP socket is closed, this is only caught when trying to write
on the output channel, which throws a `ClosedSendChannelException`.
We now catch that exception and cleanly stop the background job.

Fixes https://youtrack.jetbrains.com/issue/KTOR-5178/TLSSocket-cannot-catch-the-exception-thrown-by-appDataOutputLoop
Fixes https://youtrack.jetbrains.com/issue/KTOR-4360/Android-Impossible-to-catch-the-ClosedSendChannelException-when-TLS-connection-socket-is-closed

* fixup! Avoid crashing when TLS TCP socket is closed

(cherry picked from commit 1905329)
* KTOR-5540 Fix darwin ws pong message

* Update ktor-client/ktor-client-darwin/darwin/test/DarwinEngineTest.kt

Co-authored-by: Vitor Hugo Schwaab <vitor@schwaab.dev>

---------

Co-authored-by: Rustam <rxsinukov@gmail.com>
Co-authored-by: Vitor Hugo Schwaab <vitor@schwaab.dev>

(cherry picked from commit 375b0d3)
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
(cherry picked from commit a0ae1eb)
* KTOR-6292 Make client use Dispatchers.IO by default

(cherry picked from commit a22852c)
@e5l e5l self-assigned this Oct 2, 2023
@e5l e5l force-pushed the 2.3.5-eap branch 6 times, most recently from 61c2424 to e1a35c3 Compare October 3, 2023 10:52
* Fix mingwX64 compilation

* Update kotlin to 1.9.10

* Fix CallLogging tests

* Fix js compilation

(cherry picked from commit 8a9f4a5)
@e5l e5l marked this pull request as ready for review October 3, 2023 12:07
@e5l e5l requested review from Stexxe and marychatte October 3, 2023 12:07
@@ -8,7 +8,6 @@ public final class io/ktor/client/engine/android/AndroidClientEngine : io/ktor/c
public fun execute (Lio/ktor/client/request/HttpRequestData;Lkotlin/coroutines/Continuation;)Ljava/lang/Object;
public synthetic fun getConfig ()Lio/ktor/client/engine/HttpClientEngineConfig;
public fun getConfig ()Lio/ktor/client/engine/android/AndroidEngineConfig;
public fun getDispatcher ()Lkotlinx/coroutines/CoroutineDispatcher;
Copy link
Contributor

@Stexxe Stexxe Oct 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this an API-breaking change?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope, we dropped the override field but still have it in the base

@@ -14,6 +14,10 @@ public open class HttpClientEngineConfig {
/**
* Specifies network threads count advice.
*/
@Deprecated(
"The [threadsCount] property is deprecated. The [Dispatchers.IO] is used by default.",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it okay to have a deprecation error in a patch release?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me make it as warning

@@ -5,6 +5,7 @@ public final class io/ktor/client/plugins/logging/LogLevel : java/lang/Enum {
public static final field INFO Lio/ktor/client/plugins/logging/LogLevel;
public static final field NONE Lio/ktor/client/plugins/logging/LogLevel;
public final fun getBody ()Z
public static fun getEntries ()Lkotlin/enums/EnumEntries;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New API in a patch release

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getEntries is default enum field generated by the kotlin compiler

@@ -40,6 +40,7 @@ public final class io/ktor/http/CacheControl$NoStore : io/ktor/http/CacheControl
public final class io/ktor/http/CacheControl$Visibility : java/lang/Enum {
public static final field Private Lio/ktor/http/CacheControl$Visibility;
public static final field Public Lio/ktor/http/CacheControl$Visibility;
public static fun getEntries ()Lkotlin/enums/EnumEntries;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

new API

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same

public suspend fun readAvailable(dst: CPointer<ByteVar>, offset: Int, length: Int): Int

/**
* Reads all available bytes to [dst] buffer and returns immediately or suspends if no bytes available
* @return number of bytes were read or `-1` if the channel has been closed
*/
@OptIn(ExperimentalForeignApi::class)
public suspend fun readAvailable(dst: CPointer<ByteVar>, offset: Long, length: Long): Int
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it okay to have an OptIn annotation in a patch release?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep, it's kotlin native and triggered by the kotlin version

@e5l e5l requested a review from Stexxe October 4, 2023 07:15
Copy link
Member

@marychatte marychatte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@e5l e5l merged commit 51e7e5f into 2.3.5 Oct 4, 2023
@e5l e5l deleted the 2.3.5-eap branch October 4, 2023 10:56
marychatte added a commit to jacekgajek/ktor that referenced this pull request Sep 5, 2024
* KTOR-6221: Fix reduced concurrent reqs in Apache5 (ktorio#3738)

When using the Apache5 engine, total concurrent requests to a single
route were limited to 5 requests. This is due to the code which tried to
increase the concurrency to the ktor-standard 1000 concurrent having a
typo and setting the total max connections twice and missing the max
connections
completely.

(cherry picked from commit fa5cba1)

* Avoid crashing when TLS TCP socket is closed (ktorio#3690)

* Avoid crashing when TLS TCP socket is closed

When using a plain TCP socket with TLS, two worker jobs are spawned:

- cio-tls-input-loop
- cio-tls-output-loop

These worker jobs live in their own scope, and exceptions they throw can't
easily be caught by the caller: the only option is to install a
`CoroutineExceptionHandler` for the whole TLS socket, which is quite hacky
and may hide bugs.

When the TCP socket is closed, this is only caught when trying to write
on the output channel, which throws a `ClosedSendChannelException`.
We now catch that exception and cleanly stop the background job.

Fixes https://youtrack.jetbrains.com/issue/KTOR-5178/TLSSocket-cannot-catch-the-exception-thrown-by-appDataOutputLoop
Fixes https://youtrack.jetbrains.com/issue/KTOR-4360/Android-Impossible-to-catch-the-ClosedSendChannelException-when-TLS-connection-socket-is-closed

* fixup! Avoid crashing when TLS TCP socket is closed

(cherry picked from commit 1905329)

* KTOR-6229 Fix hostname verification in CIO (ktorio#3746)

(cherry picked from commit 53fa31a)

* KTOR-5540 Fix darwin ws pong message (ktorio#3747)

* KTOR-5540 Fix darwin ws pong message

* Update ktor-client/ktor-client-darwin/darwin/test/DarwinEngineTest.kt

Co-authored-by: Vitor Hugo Schwaab <vitor@schwaab.dev>

---------

Co-authored-by: Rustam <rxsinukov@gmail.com>
Co-authored-by: Vitor Hugo Schwaab <vitor@schwaab.dev>

(cherry picked from commit 375b0d3)

* Update netty monorepo to v4.1.97.Final (ktorio#3627)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
(cherry picked from commit a0ae1eb)

* KTOR-6292 Make client use Dispatchers.IO by default (ktorio#3748)

* KTOR-6292 Make client use Dispatchers.IO by default

(cherry picked from commit a22852c)

* Update kotlin to 1.9.10 (ktorio#3761)

* Fix mingwX64 compilation

* Update kotlin to 1.9.10

* Fix CallLogging tests

* Fix js compilation

(cherry picked from commit 8a9f4a5)

* fixup! Update kotlin to 1.9.10 (ktorio#3761)

* KTOR-6286 Update xmlutil to 0.86.2 (ktorio#3770)

(cherry picked from commit 87181e4)

---------

Co-authored-by: Sebastian Mayr <smayr@atlassian.com>
Co-authored-by: Bastien Teinturier <31281497+t-bast@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Mariia Skripchenko <61115099+marychatte@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants