Skip to content

Macos processes - #118

Closed
emanuele-em wants to merge 21 commits into
mitmproxy:mainfrom
emanuele-em:macos-processes
Closed

Macos processes#118
emanuele-em wants to merge 21 commits into
mitmproxy:mainfrom
emanuele-em:macos-processes

Conversation

@emanuele-em

@emanuele-em emanuele-em commented Oct 25, 2023

Copy link
Copy Markdown
Member

This is a first draft to get first info from running processes:

there are a lot of steps before the png image (NSImage -> tif_representation -> bitmap_representation -> png_representation), maybe we can optimize that part

@emanuele-em
emanuele-em marked this pull request as ready for review November 2, 2023 17:58
@emanuele-em
emanuele-em requested a review from mhils November 2, 2023 17:58

@mhils mhils left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Awesome work! 🚀
Fantastic to see that enumeration now works without instantiating all those NSRunningApplications, great job. 🎉

Comment thread src/processes.rs Outdated
Comment thread src/macos/icons.rs
Comment thread src/macos/icons.rs
Comment thread src/macos/processes.rs

@mhils mhils left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This looks good now! Few smaller comments inline below.

The final thing I'm worried about here is memory leaks in the unsafe code. I suppose this is not super easy to test though. At the very least, can we maybe have some (#[ignore]d by default) test cases that call the unsafe methods (e.g. tif_data_for_executable) in a loop so that one can manually check for an increase in memory? Does that make sense? Smarter ideas are welcome as well of course, but I don't want to overcomplicate things.

Comment thread src/macos/processes.rs
Comment thread Cargo.toml Outdated
Comment thread benches/process.rs Outdated
Comment thread src/macos/icons.rs Outdated
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