Skip to content

Schedule HTTP/2 re-enable on NetVConnection owner thread#13362

Open
saimayithri wants to merge 1 commit into
apache:masterfrom
saimayithri:fix-nh-queue-thread-affinity
Open

Schedule HTTP/2 re-enable on NetVConnection owner thread#13362
saimayithri wants to merge 1 commit into
apache:masterfrom
saimayithri:fix-nh-queue-thread-affinity

Conversation

@saimayithri

Copy link
Copy Markdown

Refs: #13358

Summary

Http2CommonSession::state_complete_frame_read() schedules
HTTP2_SESSION_EVENT_REENABLE on the thread currently holding the session
mutex.

That thread can differ from the underlying NetVConnection owner thread.
When the re-enable handler later processes frames or releases streams, it can
reach UnixNetVConnection queue operations from the wrong thread and trigger
the NetHandler lock assertion described in #13358.

This change schedules the re-enable event on the session NetVConnection's
owner thread instead.

Changes

At both HTTP/2 re-enable scheduling sites:

  • Keeps the existing continuation, delay, event type, and VIO unchanged.
  • Uses get_netvc()->thread as the scheduling target instead of
    get_mutex()->thread_holding.
  • Asserts that the session has an associated NetVConnection before
    scheduling.

Validation

  • Built successfully with:
    cmake --build build-dev -j4

  • Ran:
    ctest --test-dir build-dev -j4

Scope

This PR addresses the HTTP/2 re-enable path from #13358 only. The separate
HTTP/1.1 cache-write teardown path remains under investigation.

Refs: apache#13358
Signed-off-by: saimayithri <saimayithri@gmail.com>

@JosiahWI JosiahWI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The code looks good! Thank you for this fix!

@JosiahWI JosiahWI added the HTTP/2 label Jul 6, 2026
@JosiahWI JosiahWI added this to the 11.0.0 milestone Jul 6, 2026
@JosiahWI

JosiahWI commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

I'm adding this to the v10.2.x project in anticipation that it should be backported.

@masaori335

Copy link
Copy Markdown
Contributor

[approve ci]

@JosiahWI

JosiahWI commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

The AuTest subcookie spuriously failed, and the HostDBProcessor regression test failed (unsure if this is spurious).

@saimayithri saimayithri marked this pull request as ready for review July 6, 2026 13:19
@saimayithri

Copy link
Copy Markdown
Author

Thanks for the review, assignment, and for running CI !!

@JosiahWI JosiahWI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Approving, but would be good to also get a review from @wuxcer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

3 participants