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

Fix LeanFFI #246

Merged
merged 2 commits into from
Apr 8, 2024
Merged

Fix LeanFFI #246

merged 2 commits into from
Apr 8, 2024

Conversation

andrewmwells-amazon
Copy link
Contributor

@andrewmwells-amazon andrewmwells-amazon commented Mar 6, 2024

Issue #, if available:
#227

Description of changes:
Fix LeanFFI to work with multiple threads. Also decrement reference count on responses to fix memory leak.

Threads issue is blocked on leanprover/lean4#3632 being released. This PR now purely fixes the memory leak.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

cedar-drt/tests/integration_tests.rs Outdated Show resolved Hide resolved
cedar-drt/tests/benchmark.rs Outdated Show resolved Hide resolved
@khieta
Copy link
Contributor

khieta commented Mar 6, 2024

Awesome progress! Thanks for looking into this 🙂

@andrewmwells-amazon
Copy link
Contributor Author

The tests now run but Lean folks said this shouldn't work. Waiting for clarification on this thread: https://leanprover.zulipchat.com/#narrow/stream/424609-lean-at-aws/topic/Reverse.20FFI.20Thread.20Safety before we can merge.

@andrewmwells-amazon
Copy link
Contributor Author

andrewmwells-amazon commented Mar 7, 2024

Lean PR. We need to wait until we upgrade to that Lean version before we fix+merge this.

cedar-drt/src/lean_impl.rs Outdated Show resolved Hide resolved
@andrewmwells-amazon andrewmwells-amazon marked this pull request as ready for review April 5, 2024 22:15
@andrewmwells-amazon
Copy link
Contributor Author

Does not allow multiple LeanFFI threads, but should fix a memory leak.

@andrewmwells-amazon andrewmwells-amazon merged commit dab8e07 into main Apr 8, 2024
6 checks passed
@andrewmwells-amazon andrewmwells-amazon deleted the andrewmwells/lean-ffi-threading branch April 8, 2024 15:45
shaobo-he-aws pushed a commit that referenced this pull request Apr 11, 2024
shaobo-he-aws added a commit that referenced this pull request Apr 11, 2024
Signed-off-by: Shaobo He <[email protected]>
Co-authored-by: Andrew Wells <[email protected]>
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