search

vapor / postgres-nio

🐘 Non-blocking, event-driven Swift client for PostgreSQL.
https://api.vapor.codes/postgresnio/documentation/postgresnio/
MIT License
321 stars 75 forks source link

[Fix] Query Hangs if Connection is Closed #487

Closed MahdiBM closed 3 months ago

MahdiBM commented 3 months ago

See the test for how to trigger this.

MahdiBM commented 3 months ago

Cancelled the integration tests CI manually, since they wouldn't have finished.

Side note @gwynne: probably good to have a time limit in CIs.

gwynne commented 3 months ago

Side note @gwynne: probably good to have a time limit in CIs.

The default timeout is 1 hour, IIRC.

codecov[bot] commented 3 months ago

Codecov Report

Attention: Patch coverage is 75.00000% with 4 lines in your changes missing coverage. Please review.

Project coverage is 55.28%. Comparing base (7b621c1) to head (ceaa688).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #487 +/- ## ========================================== + Coverage 54.98% 55.28% +0.30% ========================================== Files 127 127 Lines 10155 10161 +6 ========================================== + Hits 5584 5618 +34 + Misses 4571 4543 -28 ``` | [Files](https://app.codecov.io/gh/vapor/postgres-nio/pull/487?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=vapor) | Coverage Δ | | |---|---|---| | [...es/PostgresNIO/Connection/PostgresConnection.swift](https://app.codecov.io/gh/vapor/postgres-nio/pull/487?src=pr&el=tree&filepath=Sources%2FPostgresNIO%2FConnection%2FPostgresConnection.swift&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=vapor#diff-U291cmNlcy9Qb3N0Z3Jlc05JTy9Db25uZWN0aW9uL1Bvc3RncmVzQ29ubmVjdGlvbi5zd2lmdA==) | `48.57% <75.00%> (+6.32%)` | :arrow_up: | ... and [3 files with indirect coverage changes](https://app.codecov.io/gh/vapor/postgres-nio/pull/487/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=vapor)
fabianfett commented 3 months ago

the test I added as an example hangs 100% of time for me.

MahdiBM commented 3 months ago

the test I added as an example hangs 100% of time for me.

Hmm you're right. I thought i tested that properly and it turned out like i said, but apparently not. I did try to remove the loop like you did with just closing the channel beforehand, but i thought that didn't work out to get the hang.

I was likely testing with my fixes in there, and not reverted.

MahdiBM commented 3 months ago

ahh hmm CI is failing with Fatal error: SWIFT TASK CONTINUATION MISUSE: listen(_:) tried to resume its continuation more than once, throwing ioOnClosedChannel! need to investigate.

MahdiBM commented 3 months ago

I reckon I didn't add a unit test for "each" modified function. Should I?

fabianfett commented 3 months ago

I reckon I didn't add a unit test for "each" modified function. Should I?

Yes, please!