Merge pull request #281 from whisperfish/unsend-hyper-fix

Make Hyper work with unsend futures
whisperfish · Jan 12, 2024 · c3c6a12 · c3c6a12
2 parents 0b194b2 + 699c68c
commit c3c6a12
Show file tree

Hide file tree

Showing 2 changed files with 26 additions and 5 deletions.
diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml
@@ -23,12 +23,17 @@ jobs:
       fail-fast: false
       matrix:
         project: ["libsignal-service-actix", "libsignal-service-hyper", "libsignal-service"]
+        features: ["", "unsend-futures"]
+        exclude:
+          # -actix always has unsend futures, so we don't have that feature flag
+          - project: "libsignal-service-actix"
+            features: "unsend-futures"
     steps:
       - uses: actions/checkout@v3
       - uses: actions-rs/clippy-check@v1
         with:
           token: ${{ secrets.GITHUB_TOKEN }}
-          args: --manifest-path ${{ matrix.project }}/Cargo.toml
+          args: --features "${{ matrix.features }}" --manifest-path ${{ matrix.project }}/Cargo.toml
 
   build:
     name: Build (${{ matrix.project }}, Rust ${{ matrix.toolchain }})
@@ -39,13 +44,25 @@ jobs:
         project: ["libsignal-service-actix", "libsignal-service-hyper", "libsignal-service"]
         toolchain: ["stable", "beta", "nightly"]
         coverage: [false, true]
+        features: ["", "unsend-futures"]
         exclude:
+          # Coverage related excludes
           - toolchain: stable
             coverage: true
           - toolchain: beta
             coverage: true
           - toolchain: nightly
             coverage: false
+
+          # Feature flag related excludes
+            # Actix like above
+          - project: "libsignal-service-actix"
+            features: "unsend-futures"
+            # We don't need to spawn this many jobs to see that unsend-futures works
+          - features: "unsend-futures"
+            toolchain: "beta"
+          - features: "unsend-futures"
+            toolchain: "nightly"
         include:
           - project: "libsignal-service-actix"
             toolchain: "1.70"
@@ -62,20 +79,20 @@ jobs:
         if: ${{ !matrix.coverage }}
         with:
           command: test
-          args: --all-targets --no-fail-fast --manifest-path ${{ matrix.project }}/Cargo.toml
+          args: --all-targets --no-fail-fast --features "${{ matrix.features }}" --manifest-path ${{ matrix.project }}/Cargo.toml
 
       - name: Build
         uses: actions-rs/cargo@v1
         with:
           command: build
-          args: --all-targets --manifest-path ${{ matrix.project }}/Cargo.toml
+          args: --all-targets --features "${{ matrix.features }}" --manifest-path ${{ matrix.project }}/Cargo.toml
 
       - name: Run tests
         uses: actions-rs/cargo@v1
         if: ${{ matrix.coverage }}
         with:
           command: test
-          args: --all-targets --no-fail-fast --manifest-path ${{ matrix.project }}/Cargo.toml
+          args: --all-targets --no-fail-fast --features "${{ matrix.features }}" --manifest-path ${{ matrix.project }}/Cargo.toml
         env:
           CARGO_INCREMENTAL: '0'
           RUSTFLAGS: '-Zprofile -Ccodegen-units=1 -Cinline-threshold=0 -Clink-dead-code -Coverflow-checks=off -Cpanic=abort -Zpanic_abort_tests'

diff --git a/libsignal-service-hyper/src/push_service.rs b/libsignal-service-hyper/src/push_service.rs
@@ -611,7 +611,11 @@ impl PushService for HyperPushService {
         .await?;
         let (ws, task) =
             SignalWebSocket::from_socket(ws, stream, keepalive_path.to_owned());
-        tokio::task::spawn(task.instrument(span));
+        let task = task.instrument(span);
+        #[cfg(feature = "unsend-futures")]
+        tokio::task::spawn_local(task);
+        #[cfg(not(feature = "unsend-futures"))]
+        tokio::task::spawn(task);
         Ok(ws)
     }
 }