Skip to content

fix(job): fix lambda closure capture and init failure tracking#4832

Open
SezginKahraman wants to merge 1 commit intolivekit:mainfrom
SezginKahraman:fix/job-closure-and-init-tracking
Open

fix(job): fix lambda closure capture and init failure tracking#4832
SezginKahraman wants to merge 1 commit intolivekit:mainfrom
SezginKahraman:fix/job-closure-and-init-tracking

Conversation

@SezginKahraman
Copy link
Contributor

  • Fix race condition in participant task cleanup where lambda captured p.identity by reference instead of by value, causing the callback to use a potentially stale identity if the participant object changes
  • Track job initialization failures in ThreadJobExecutor: log warnings for timeout/exception cases and set job status to FAILED instead of incorrectly reporting SUCCESS

- Fix race condition in participant task cleanup where lambda captured
  p.identity by reference instead of by value, causing the callback to
  use a potentially stale identity if the participant object changes
- Track job initialization failures in ThreadJobExecutor: log warnings
  for timeout/exception cases and set job status to FAILED instead of
  incorrectly reporting SUCCESS

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link
Contributor

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 3 additional findings.

Open in Devin Review

Copy link
Member

@davidzhao davidzhao left a comment

Choose a reason for hiding this comment

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

the job status change makes sense. could you run make fix to clean up the formatting issues?

task = asyncio.create_task(coro(self, p), name=task_name)
self._participant_tasks[(p.identity, coro)] = task
task.add_done_callback(
lambda _, coro=coro: self._participant_tasks.pop((p.identity, coro)) # type: ignore
Copy link
Member

Choose a reason for hiding this comment

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

I don't think it's possible for a particular participant obj to change its identity.. why is this needed?

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.

2 participants