Skip to content

Conversation

@ValentaTomas
Copy link
Member

@ValentaTomas ValentaTomas commented Nov 3, 2025

Note

Replace dirty-page tracking with a thread-safe bitset Tracker, add UFFD unregister support, and make memory backend Disable/Dirty context-aware across sandbox and UFFD.

  • Block tracking:
    • Introduce block.Tracker (thread-safe bitset) with Add, Has, Reset, BitSet, Clone, BlockSize; add comprehensive tests.
  • UFFD integration:
    • Replace sync.Map/TrackedSliceDevice with Tracker for missingRequests; use Add to de-dup faults.
    • Add unregister support via UFFDIO_UNREGISTER and unregister(); expose Userfaultfd.Unregister() and wire through Uffd.Disable(ctx).
    • Update constructors to pass page size; adjust message handling and tests to read offsets from Tracker.
  • API changes:
    • Update MemoryBackend to Disable(ctx) and Dirty(ctx) (*block.Tracker, error); migrate NoopMemory and Uffd accordingly.
  • Sandbox:
    • Use new memory.Disable(ctx) and memory.Dirty(ctx); pass dirty.BitSet() to memory diff creation.

Written by Cursor Bugbot for commit 116062b. This will update automatically on new commits. Configure here.

@ValentaTomas
Copy link
Member Author

I want to do a few more end to end tests, but it should be ready to merge right after https://github.com/e2b-dev/infra/tree/uffd-refactor

@ValentaTomas
Copy link
Member Author

@codex review

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

msg := *(*UffdMsg)(unsafe.Pointer(&buf[0]))
if GetMsgEvent(&msg) != UFFD_EVENT_PAGEFAULT {
u.logger.Error("UFFD serve unexpected event type", zap.Any("event_type", GetMsgEvent(&msg)))
return ErrUnexpectedEventType

P1 Badge Handle UFFD remove events after unregister

The new Disable(ctx) issues UFFDIO_UNREGISTER, which causes the kernel to emit UFFD_EVENT_REMOVE notifications on the userfaultfd. The serve loop still treats every non‑UFFD_EVENT_PAGEFAULT as an error and immediately returns ErrUnexpectedEventType. After disabling memory this will report a failure, tear down the goroutine and mark the backend as errored even though the event is expected. The loop should explicitly ignore or handle UFFD_EVENT_REMOVE (and possibly other control events) once unregistering is supported.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@ValentaTomas
Copy link
Member Author

💡 Codex Review

msg := *(*UffdMsg)(unsafe.Pointer(&buf[0]))
if GetMsgEvent(&msg) != UFFD_EVENT_PAGEFAULT {
u.logger.Error("UFFD serve unexpected event type", zap.Any("event_type", GetMsgEvent(&msg)))
return ErrUnexpectedEventType

P1 Badge Handle UFFD remove events after unregister
The new Disable(ctx) issues UFFDIO_UNREGISTER, which causes the kernel to emit UFFD_EVENT_REMOVE notifications on the userfaultfd. The serve loop still treats every non‑UFFD_EVENT_PAGEFAULT as an error and immediately returns ErrUnexpectedEventType. After disabling memory this will report a failure, tear down the goroutine and mark the backend as errored even though the event is expected. The loop should explicitly ignore or handle UFFD_EVENT_REMOVE (and possibly other control events) once unregistering is supported.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Isn't UFFD_EVENT_REMOVE triggered by MADV_DONTNEED/MADV_REMOVE, which are not actually part of the UFFD unregister signalization—they are part of the UFFD memory management which we are not using. Not 100% sure though.

@chatgpt-codex-connector
Copy link

Summary

Testing

  • ⚠️ go test ./packages/orchestrator/internal/sandbox/uffd/... (fails: go: command not found)

View task →

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