Skip to content

fix(httpweb): resolve pending request future on disconnect - #808

Merged
re2zero merged 1 commit into
linuxdeepin:masterfrom
re2zero:pms396845/05-http-future-v2
Oct 22, 2026
Merged

re2zero merged 1 commit into
linuxdeepin:masterfrom
re2zero:pms396845/05-http-future-v2

Conversation

@re2zero

@re2zero re2zero commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

Part 5/8 of the reverse tunnel stack (recovery of #801).

This reapplies the reviewed commit on top of the corrected stack because #801 was accidentally merged into its stacked base branch instead of master.

Merge order: #805 → #806 → #807 → #808 → #809 → #810 → #811.
Retargeted to master after #807 merged.

HTTPSClientEx canceled its timeout timer on disconnect without completing
the request promise. FileClient::downloadFile() could then block forever
in future::get(), and FileClient's destructor froze the GUI while joining
the download thread on cancel.

Fail the pending promise on disconnect and guard promise completion so a
late response cannot satisfy it twice.

Log: resolve pending request future on disconnect
PMS: TASK-396845
Influence: cancelling a download no longer blocks the GUI while joining the download thread.

Summary by Sourcery

Ensure disconnected HTTPS requests terminate promptly so canceled downloads do not freeze the application.

Bug Fixes:

  • Resolve pending HTTP request futures with an error when the connection disconnects, preventing canceled downloads from blocking indefinitely.
  • Prevent late responses or timeout/disconnect races from completing the same request promise more than once.

Tests:

  • Add coverage verifying that a disconnected HTTPS request becomes ready and reports an error.

@sourcery-ai

sourcery-ai Bot commented Oct 21, 2026

Copy link
Copy Markdown

Reviewer's Guide

Updates HTTPSClientEx to fail pending request futures on disconnect and atomically prevent timeout, disconnect, and late-response paths from completing the promise more than once. Adds a local-server regression test confirming canceled/disconnected requests no longer leave synchronous callers blocked indefinitely.

Sequence diagram for resolving HTTP requests on disconnect

sequenceDiagram
    participant Caller as FileClient::downloadFile
    participant Client as HTTPSClientEx
    participant Connection as HTTPSConnection
    participant Future as RequestFuture

    Caller->>Client: SendRequest(request)
    Client->>Future: create pending promise
    Client->>Connection: send request
    Connection--xClient: onDisconnected()
    Client->>Connection: Cancel timeout
    Client->>Client: SetPromiseError(Disconnected!)
    Client->>Future: set_exception
    Future-->>Caller: future.get() throws

    alt late response or timeout
        Client->>Client: SetPromiseValue(response) or SetPromiseError(error)
        Client-->>Client: return because promise already completed
    end
Loading

File-Level Changes

Change Details Files
Ensure disconnected HTTPS requests always resolve their futures and prevent duplicate promise completion during races.
  • Track whether the request promise is still pending with an atomic flag.
  • Fail the pending promise when disconnect cancels the timeout, after allowing partial-response handling to run.
  • Guard both success and error completion paths against late responses or competing timeout/disconnect handlers.
  • Reset the pending state when starting each request.
src/infrastructure/netutil/include/http/https_client.h
src/infrastructure/netutil/src/http/https_client.cpp
Add an integration-style regression test for disconnecting before a response arrives.
  • Start a local TCP server that accepts and aborts connections before TLS completes.
  • Verify the request future becomes ready well before its timeout and contains a runtime error.
  • Clean up the client and asynchronous service after the assertion.
tests/manager/httpclient_test.cpp

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hey - I've reviewed your changes and they look great!


Sourcery is free for open source - if you like our reviews please consider sharing them ✨

lzwind
lzwind previously approved these changes Oct 22, 2026
@deepin-ci-robot

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: lzwind, re2zero

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@re2zero
re2zero force-pushed the pms396845/05-http-future-v2 branch from c247b1f to 95bb6c7 Compare October 22, 2026 07:54
@re2zero
re2zero force-pushed the pms396845/04-stream-ids-v2 branch from 4e98d8f to 8e31bef Compare October 22, 2026 07:55
@re2zero
re2zero force-pushed the pms396845/05-http-future-v2 branch from 95bb6c7 to 61297ad Compare October 22, 2026 08:18
@re2zero
re2zero force-pushed the pms396845/04-stream-ids-v2 branch from 8e31bef to 5e03963 Compare October 22, 2026 08:18
@deepin-ci-robot

Copy link
Copy Markdown

deepin pr auto review

🤖 AI 代码审查报告

总体评分: 98 分 (通过阈值: 70分)

Pass


📊 总体评价

项目 结果
审查结论 代码审查通过
评分详情 本次提交修复了 HTTPSClientEx 在连接断开时未 resolve pending future 导致 FileClient::downloadFile() 阻塞、应用卡死的缺陷。通过引入 std::atomic 守卫防止 promise 重复完成,并在 onDisconnected 中补充 SetPromiseError 调用, 逻辑正确、注释充分、附有完整测试。唯一改进点为测试代码缺少 RAII 清理守卫, ASSERT_EQ 失败时可能跳过 service->Stop() 导致 std::terminate。

🔍 详细分析

1. 语法逻辑 ✅

评价: 语法正确,逻辑清晰 ✅ 通过

潜在问题:
✅ 未发现明显问题

建议: ['无需修改']


2. 代码质量 ✅

评价: 代码结构清晰,注释完整 ✅ 通过

潜在问题:
✅ 未发现明显问题

建议: ['在测试中使用 RAII scope guard(如 struct ServiceGuard { std::shared_ptr& svc; ~ServiceGuard() { svc->Stop(); } } guard{service};)或改用 gtest fixture 的 TearDown() 方法,确保 service->Stop() 在任何路径下都被调用']


3. 代码性能 ✅

评价: 性能良好,资源使用合理 ✅ 通过

潜在问题:
✅ 未发现明显问题

建议: ['无需修改']


4. 代码安全 🔒

评价: 存在0个安全漏洞 ✅ 通过

🔐 发现 0 个安全漏洞

安全漏洞详情:
✅ 未发现安全漏洞

建议: ['无需修改']


💡 改进建议代码示例

// tests/manager/httpclient_test.cpp — 使用 RAII 守卫确保清理
TEST(HTTPClientExTest, DisconnectResolvesPendingRequest)
{
    QTcpServer server;
    ASSERT_TRUE(server.listen(QHostAddress::LocalHost, 0));
    QObject::connect(&server, &QTcpServer::newConnection, &server, [&server] {
        while (QTcpSocket *socket = server.nextPendingConnection())
            socket->abort();
    });

    auto service = std::make_shared<AsioService>();
    service->Start();

    // RAII guard: ensures service->Stop() is always called,
    // even if an ASSERT_* fails and returns early.
    struct ServiceGuard {
        std::shared_ptr<AsioService>& svc;
        ~ServiceGuard() { svc->Stop(); }
    } guard{service};

    auto client = std::make_shared<NetUtil::HTTP::HTTPSClientEx>(
            service, SecureConfig::clientContext(),
            "127.0.0.1", server.serverPort());
    auto future = client->SendGetRequest("download/payload.txt&token=test",
                                         BaseKit::Timespan::seconds(30));

    for (int i = 0; i < 50
         && future.wait_for(std::chrono::milliseconds(0)) != std::future_status::ready;
         ++i) {
        QTest::qWait(100);
    }

    ASSERT_EQ(future.wait_for(std::chrono::milliseconds(0)), std::future_status::ready);
    EXPECT_THROW(future.get(), std::runtime_error);

    client->DisconnectAsync();
    client.reset();
    // service->Stop() 由 ServiceGuard 析构自动调用
}

本报告由 AI 代码审查工具自动生成

HTTPSClientEx canceled its timeout timer on disconnect without completing
the request promise. FileClient::downloadFile() could then block forever
in future::get(), and FileClient's destructor froze the GUI while joining
the download thread on cancel.

Fail the pending promise on disconnect and guard promise completion so a
late response cannot satisfy it twice.

Log: resolve pending request future on disconnect
PMS: TASK-396845
Influence: cancelling a download no longer blocks the GUI while joining the download thread.
@re2zero
re2zero force-pushed the pms396845/05-http-future-v2 branch from 61297ad to 4dc4ef9 Compare October 22, 2026 08:46
@re2zero
re2zero changed the base branch from pms396845/04-stream-ids-v2 to master October 22, 2026 08:46
@re2zero
re2zero dismissed lzwind’s stale review October 22, 2026 08:46

The base branch was changed.

@re2zero
re2zero merged commit 02f6b39 into linuxdeepin:master Oct 22, 2026
20 checks passed
@re2zero
re2zero deleted the pms396845/05-http-future-v2 branch October 22, 2026 08:57
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.

3 participants