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

Runtime: reduce memory leak caused by channels #1581

Merged
merged 2 commits into from
Mar 4, 2024
Merged

Conversation

hhugo
Copy link
Member

@hhugo hhugo commented Mar 4, 2024

No description provided.

@tkluck-janestreet
Copy link

This was my first version too 😇

I'm looking back in review history and I think we ended up with #1577 because it separates the code we want to keep (with references being traced by the GC and nothing really depends on closing fds) from the code we eventually don't (overrides that can't be traced for ppx_expect compatibility).

But that depends on a possibly faulty model for how you'd like this code to develop. If you'd rather go with #1581 and/or with moving overrides to the fd, then I think that's all a very reasonable way forward.

@hhugo hhugo merged commit 7c886d2 into master Mar 4, 2024
15 checks passed
@hhugo hhugo deleted the reduce-leak-chan branch March 4, 2024 21:15
@hhugo
Copy link
Member Author

hhugo commented Mar 4, 2024

Merging this one first, giving more time to properly fix the leak.

@tkluck-janestreet
Copy link

Thanks @hhugo!

mseri pushed a commit to ocaml/opam-repository that referenced this pull request Mar 5, 2024
CHANGES:

## Features/Changes
* Compiler: only flush the necessary env for closures (ocsigen/js_of_ocaml#1568)
* Library: dialog element support

## Bug fixes
* Compiler: fix --enable=vardecl
* Compiler: fix parallel renaming (bug introduced in ocsigen/js_of_ocaml#1567)
* Lib: fix paragraph construction and coercion
* Runtime: reduce memory leak with channels (ocsigen/js_of_ocaml#1581)
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.

None yet

2 participants