Skip to content

Commit

Permalink
proxy: fix stack overflow in cancel publisher (#7212)
Browse files Browse the repository at this point in the history
## Problem

stack overflow in blanket impl for `CancellationPublisher`

## Summary of changes

Removes `async_trait` and fixes the impl order to make it non-recursive.
  • Loading branch information
conradludgate authored Mar 23, 2024
1 parent 643683f commit 72103d4
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 15 deletions.
15 changes: 15 additions & 0 deletions proxy/src/cancellation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -211,4 +211,19 @@ mod tests {

Ok(())
}

#[tokio::test]
async fn cancel_session_noop_regression() {
let handler = CancellationHandler::<()>::new(Default::default(), "local");
handler
.cancel_session(
CancelKeyData {
backend_pid: 0,
cancel_key: 0,
},
Uuid::new_v4(),
)
.await
.unwrap();
}
}
24 changes: 9 additions & 15 deletions proxy/src/redis/cancellation_publisher.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
use std::sync::Arc;

use async_trait::async_trait;
use pq_proto::CancelKeyData;
use redis::AsyncCommands;
use tokio::sync::Mutex;
Expand All @@ -13,47 +12,44 @@ use super::{
notifications::{CancelSession, Notification, PROXY_CHANNEL_NAME},
};

#[async_trait]
pub trait CancellationPublisherMut: Send + Sync + 'static {
#[allow(async_fn_in_trait)]
async fn try_publish(
&mut self,
cancel_key_data: CancelKeyData,
session_id: Uuid,
) -> anyhow::Result<()>;
}

#[async_trait]
pub trait CancellationPublisher: Send + Sync + 'static {
#[allow(async_fn_in_trait)]
async fn try_publish(
&self,
cancel_key_data: CancelKeyData,
session_id: Uuid,
) -> anyhow::Result<()>;
}

#[async_trait]
impl CancellationPublisherMut for () {
impl CancellationPublisher for () {
async fn try_publish(
&mut self,
&self,
_cancel_key_data: CancelKeyData,
_session_id: Uuid,
) -> anyhow::Result<()> {
Ok(())
}
}

#[async_trait]
impl<P: CancellationPublisherMut> CancellationPublisher for P {
impl<P: CancellationPublisher> CancellationPublisherMut for P {
async fn try_publish(
&self,
_cancel_key_data: CancelKeyData,
_session_id: Uuid,
&mut self,
cancel_key_data: CancelKeyData,
session_id: Uuid,
) -> anyhow::Result<()> {
self.try_publish(_cancel_key_data, _session_id).await
<P as CancellationPublisher>::try_publish(self, cancel_key_data, session_id).await
}
}

#[async_trait]
impl<P: CancellationPublisher> CancellationPublisher for Option<P> {
async fn try_publish(
&self,
Expand All @@ -68,7 +64,6 @@ impl<P: CancellationPublisher> CancellationPublisher for Option<P> {
}
}

#[async_trait]
impl<P: CancellationPublisherMut> CancellationPublisher for Arc<Mutex<P>> {
async fn try_publish(
&self,
Expand Down Expand Up @@ -145,7 +140,6 @@ impl RedisPublisherClient {
}
}

#[async_trait]
impl CancellationPublisherMut for RedisPublisherClient {
async fn try_publish(
&mut self,
Expand Down

1 comment on commit 72103d4

@github-actions
Copy link

Choose a reason for hiding this comment

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

2798 tests run: 2649 passed, 1 failed, 148 skipped (full report)


Failures on Postgres 14

  • test_bulk_insert[neon-github-actions-selfhosted]: release
# Run all failed tests locally:
scripts/pytest -vv -n $(nproc) -k "test_bulk_insert[neon-release-pg14-github-actions-selfhosted]"

Code coverage* (full report)

  • functions: 28.2% (6290 of 22336 functions)
  • lines: 46.9% (44191 of 94150 lines)

* collected from Rust tests only


The comment gets automatically updated with the latest test results
72103d4 at 2024-03-23T07:47:23.778Z :recycle:

Please sign in to comment.