Skip to content
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

Fixed memory leak on Apple platforms #17

Merged
merged 1 commit into from
Feb 15, 2024

Conversation

aw-cf
Copy link
Contributor

@aw-cf aw-cf commented Feb 1, 2024

Fixes #16.

  • Avoids the unnecessary allocation of thread_state
  • Ensures the mach port rights, as well as the thread list is deallocated

@aw-cf aw-cf force-pushed the fixed-apple-memory-leak branch from 3de4831 to 6178662 Compare February 1, 2024 12:18
@jhpratt
Copy link
Owner

jhpratt commented Feb 1, 2024

CI is failing with a linker error. This may not be related, as the most recent commit on main is a merged PR that attempted to address this. cc @Freaky if you recall anything?

@Freaky
Copy link
Collaborator

Freaky commented Feb 1, 2024

Rust 1.54 fixed a compile issue with Xcode 14, perhaps we need to pin that to 13 with something like:

run: sudo xcode-select -s '/Applications/Xcode_13.4.1.app'

@thibault-ml
Copy link
Contributor

thibault-ml commented Feb 1, 2024

Is it not possible to use the latest Rust (1.75) + latest Xcode (15.2)?

@thibault-ml
Copy link
Contributor

@Freaky @jhpratt is there any way we can help here? :-)

@jhpratt
Copy link
Owner

jhpratt commented Feb 7, 2024

With CI failing, I have no idea. I'm not familiar with Mac APIs.

@thibault-ml
Copy link
Contributor

@jhpratt I've created a PR to update the Rust version, which should fix compiling on macOS: #20

@thibault-ml
Copy link
Contributor

@jhpratt @Freaky now that we upped the MSRV, I believe this PR should build. Can we rerun the builds?

@jhpratt
Copy link
Owner

jhpratt commented Feb 11, 2024

The current CI config doesn't let me queue a manual run. If you run git commit --amend --no-edit && git push -f when nothing is staged, it has the same effect as queueing a new CI run.

@aw-cf aw-cf force-pushed the fixed-apple-memory-leak branch from 6178662 to 525f669 Compare February 11, 2024 16:07
@thibault-ml
Copy link
Contributor

Nice, looks like we're all green :-)

@jhpratt @Freaky Anything else you need?

@jhpratt jhpratt merged commit 3cfc527 into jhpratt:main Feb 15, 2024
9 checks passed
@jhpratt
Copy link
Owner

jhpratt commented Feb 15, 2024

LGTM! Merged and releasing momentarily.

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.

Memory leak in num_threads::is_single_threaded() on Apple platforms
4 participants