Skip to content

Conversation

@d-torrance
Copy link
Member

The GC finalizer callback function expects two parameters: a pointer to the GC memory we're finalizing and a pointer to some client data. We were previously passing the first object to mpz_clear and mpfr_clear, which was incorrect. (This is why the mpfr code was commented out, as otherwise we were segfaulting trying to call mpfr_clear on our pointer object rather than on the mpfr_ptr it's pointing to!. I'm surprised the gmp code wasn't segfaulting too!)

The GC finalizer callback function expects two parameters: a pointer
to the GC memory we're finalizing and a pointer to some client data.
We were previously passing the *first* object to mpz_clear and
mpfr_clear, which was incorrect.  (This is why the mpfr code was
commented out, as otherwise we were segfaulting trying to call
mpfr_clear on our pointer object rather than on the mpfr_ptr it's
pointing to!.  I'm surprised the gmp code wasn't segfaulting too!)
@d-torrance d-torrance requested a review from antonleykin January 6, 2026 13:57
@mahrud
Copy link
Member

mahrud commented Jan 6, 2026

I'm a bit confused, did mpfr just not finalize at all before? Is it possible to add some sort of test or memory benchmark to compare with previous performance?

@d-torrance
Copy link
Member Author

That's correct -- I guess I gave up too soon trying to figure out why it was segfaulting...

I just added a test that creates a bunch of these objects and then collects the garbage. Is that the kind of thing you were asking for? I'm not sure of a good way to check if we actually finalized the things we wanted to.

@jkyang92
Copy link
Contributor

jkyang92 commented Jan 7, 2026

This applies to ffi related objects only right? Everything else the engine does that I know of reallocates the limbs into gc memory.

@d-torrance
Copy link
Member Author

Yes, that's correct. Only objects of class mpzT and mpfrT constructed using the ForeignFunctions package.

@d-torrance
Copy link
Member Author

Discussed with Mike

@d-torrance d-torrance merged commit a66365a into Macaulay2:development Jan 20, 2026
5 checks passed
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