Skip to content

internal/sources/windows: copy from kawa #14

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Apr 16, 2025

Conversation

zombiezen
Copy link
Contributor

In the process, I made some improvements:

  • syscall.NewCallback is a limited resource, but it gets called on every subscription. For event log-heavy configurations, this could crash reveald. To address this, I create a single callback at package initialization.
  • Rather than make the entire event log package Windows-only, I stub out the relevant system calls on non-Windows platforms. This allows compiling the source files and API on all platforms, regardless of support.
  • I discovered that slow receivers can cause events to be dropped. I introduced an optional buffer to the channel to handle bursts of events. A small buffer is added by default.
  • To ensure this source works, I added a simple end-to-end test.

Updates runreveal/kawa#61

In the process, I made some improvements:

- `syscall.NewCallback` is a limited resource,
  but it gets called on every subscription.
  For event log-heavy configurations,
  this could crash reveald.
  To address this, I create a single callback at package initialization.
- Rather than make the entire event log package Windows-only,
  I stub out the relevant system calls on non-Windows platforms.
  This allows compiling the source files and API on all platforms,
  regardless of support.
- I discovered that slow receivers can cause events to be dropped.
  I introduced an optional buffer to the channel to handle bursts of events.
  A small buffer is added by default.
- To ensure this source works, I added a simple end-to-end test.

Updates runreveal/kawa#61
@zombiezen zombiezen requested a review from abraithwaite April 9, 2025 21:46
Copy link
Member

@abraithwaite abraithwaite left a comment

Choose a reason for hiding this comment

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

Thank you for finding the little issues with this package! Really appreciate that and adding a test for it.

I kindly request that we return to the Runner pattern though. I know can be a bit awkward to use at first but maintaining consistency is important for us. I can help with any testing issues you've run into if you can show me an example of where it fell short. :-)

@zombiezen
Copy link
Contributor Author

Done. PTAL

Copy link
Member

@abraithwaite abraithwaite left a comment

Choose a reason for hiding this comment

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

Thank you! Appreciate the understanding around io.Closer.

@zombiezen zombiezen merged commit 1d27ae4 into runreveal:main Apr 16, 2025
1 check passed
@zombiezen zombiezen deleted the windows-event-log branch April 16, 2025 17:16
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