Skip to content

Optional Recursive Mutex in Asio Integration - #1846

Open
RobertLeahy wants to merge 3 commits into
NVIDIA:mainfrom
RobertLeahy:optional_recursive_mutex_20260214
Open

RobertLeahy wants to merge 3 commits into
NVIDIA:mainfrom
RobertLeahy:optional_recursive_mutex_20260214

Conversation

@RobertLeahy

Copy link
Copy Markdown
Contributor

Please do not squash.

Resolves #1781.

@copy-pr-bot

copy-pr-bot Bot commented Feb 14, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@ericniebler ericniebler left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

nit picks

Comment thread include/asioexec/completion_token.hpp Outdated
Comment thread include/asioexec/completion_token.hpp Outdated
Comment thread include/asioexec/completion_token.hpp Outdated
Comment thread include/asioexec/completion_token.hpp Outdated
Comment thread include/asioexec/completion_token.hpp Outdated
Comment thread include/asioexec/completion_token.hpp Outdated
Comment thread include/asioexec/use_sender.hpp Outdated
@ericniebler

Copy link
Copy Markdown
Collaborator

/ok to test 72c179e

@ericniebler

Copy link
Copy Markdown
Collaborator

@RobertLeahy did you see my comments? also, this PR needs a rebase.

@RobertLeahy

Copy link
Copy Markdown
Contributor Author

@RobertLeahy did you see my comments? also, this PR needs a rebase.

Yes, just haven't looped back to incorporating any of the feedback.

@RobertLeahy
RobertLeahy force-pushed the optional_recursive_mutex_20260214 branch from 72c179e to 6ffbd0a Compare August 1, 2026 16:45
@ericniebler

Copy link
Copy Markdown
Collaborator

/ok to test 6ffbd0a

@ericniebler

Copy link
Copy Markdown
Collaborator

Needs merge conflict resolution

Reduces the size of a "frame" by removing the std::unique_lock member
variable and making the "frame" itself the lock guard. This is the lock
management method shown when presenting exec::asio::completion_token in
the CppCon 2025 talk "std::execution in Asio Codebases: Adopting Senders
Without a Rewrite."
For general purpose (i.e. potentially multithreaded) use the
asynchronous operations which result when passing the exec::asio::
completion_token and ::use_sender completion tokens must use a recursive
mutex internally. However if the user knows that no multithreaded use
will occur this recursive mutex is pure overhead. Provided the
exec::asio::thread_unsafe_completion_token and _use_sender completion
tokens which do not make use of a recursive mutex for the aforementioned
use case.
@RobertLeahy
RobertLeahy force-pushed the optional_recursive_mutex_20260214 branch from 6ffbd0a to 51d2dad Compare August 9, 2026 22:19
@ericniebler

Copy link
Copy Markdown
Collaborator

/ok to test 51d2dad

@jessmorecroft

Copy link
Copy Markdown

looking forward to this one! 🙏

MikeMarcin added a commit to MikeMarcin/BetterPullReview that referenced this pull request Sep 12, 2026
…s and the corrected Visual Assist reading on BPR-0738

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
MikeMarcin added a commit to MikeMarcin/BetterPullReview that referenced this pull request Sep 12, 2026
Six candidates at app-release-e4fdb0ea-2026-09-11.4 on the tests-disabled
signed bundle built at 3543824 (BPR-0646 and BPR-0720 re-asked on
NVIDIA/stdexec#1846; BPR-0734, BPR-0737, BPR-0738, BPR-0739), ready and
protected at sequence 341; no host started. The six open cards carry the
v008 pointer and BPR-0593 the d18 frontier.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
MikeMarcin added a commit to MikeMarcin/BetterPullReview that referenced this pull request Sep 12, 2026
Every verdict is recorded against its own candidate and fingerprint; all
six match the v008 identity file, so no verdict is stale.

BPR-0646: the handle is unjudged for the third round, and not because of
the pull request. An epoch is this install's own observation and nothing
enumerates a pull request's earlier updates, so a first open yields one
epoch for any pull request; the local catalogue holds exactly one row for
NVIDIA/stdexec#1846, minted five minutes before the verdict. BPR-0743
holds the owner's choice between importing history and rewording the
criterion.

BPR-0720 and BPR-0738 are one requirement in two notes: a directive line
paints grey end to end, where the reference greys the directive, paints
its ALL_CAPS argument as a macro and its include path as a string.
BPR-0744. The include-path question is answered; the alias italic and
the CE8AD4-for-BD63C5 substitution are not, and are recorded as still
open rather than read into "most things are good now".

BPR-0734: the one-pane map is the link map at its limit, not a new
element, and the RFC's non-goal is relaxed for that direction. BPR-0745
redraws the state. Block order and where the choice lives stay
unanswered and are asked again.

BPR-0739: the seam is accepted, the cursor is not. The highlight is
painted only from underMouse(), so the handle held Qt's pointer state
while the cursor stayed an arrow, which refutes the d17 cause. BPR-0746
carries the cursor-delivery hypotheses and the check that discriminates
them on the real application. AC1's cocoa-probe sentence is corrected:
that log has the pointer 1164 px from the handle and prints FAILED.

All five cards stay parked on human-review.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
MikeMarcin added a commit to MikeMarcin/BetterPullReview that referenced this pull request Sep 12, 2026
The signed-out obstacle does not exist. Run rather than reasoned about: GET
/repos/NVIDIA/stdexec/issues/1846/timeline is HTTP 200 with no credential,
against the 60-per-hour unauthenticated budget. The timeline names both force
pushes with GitHub-side ids and the head each moved to, and one of those heads
-- 6ffbd0ac -- appears nowhere in the commits list, so this card's claim that
commits cannot reconstruct the updates that matter is now demonstrated rather
than asserted. That head is still fetchable, so an import would not list
updates it cannot then diff.

Three limits came out of the same response and the design is built around
them: the REST force-push event has no before-commit (GraphQL has it and is
refused anonymously), array position is not an ordering key and `committed`
dates are author dates, and an ordinary push is not an event at all.

Decision 1, the derivation: a digest over the authority scope and the update's
head OID, carried beside review_epoch_id. Not the event node_id, which exists
only for force pushes and which catalog_database.cpp:86-88 excludes by design;
and not the base/head pair this card offered as the fallback, because the base
is time-dependent even for a live read.

Decision 2, the ordinal: not redefined. Redefining it cannot buy portability
while Incarnation stays in the id, two of its seven serializations are already
outside the catalogue's reach (git refs on disk, MCP bindings and agent launch
JSON already handed out), and density is an adapter guarantee that history
paging and alias resolution have come to depend on.

Decision 3 is the honest limit: identity can be made stable, population cannot
be made equal. A head observed live and then force-pushed over is recoverable
by nobody, which is precisely why the identity must not be the ordinal.

Decision 4 is where it exceeds the card. A base OID is mandatory and
origin-blind, and the timeline supplies none for a historical update.

So the conditional fires, on kind rather than size: identity, base OID,
observed time and the schema version all move, four of the six things
RFC-BPR-0003 pins. Nothing is implemented. The ask is one question -- its own
RFC, or an amendment against the closed one -- and it is on the card.

Three of the card's anchors were wrong and are corrected in place: the
anonymous admission header's path, the ordinal id write line, and the
sixteen-hex-digit ref encoding, which is in installed_git_process.cpp and not
installed_git_cache.cpp.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

This branch has not been deployed

No deployments
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.

Recursive Mutex in Asio Integration Should Be Optional

3 participants