search

flatironinstitute / finufft

Non-uniform fast Fourier transform library of types 1,2,3 in dimensions 1,2,3
Other
300 stars 79 forks source link

phihat1, phihat2 and phihat3 are not correctly freed in some error cases resulting in memory leaks #547

Closed DiamonDinoia closed 1 week ago

DiamonDinoia commented 2 months ago

This problem will once we de macroize and we use stl containers instead of malloc/free.

ahbarnett commented 2 months ago

Is this type 3 only? CPU or GPU? Etc. Can you make a concrete proposal?

DiamonDinoia commented 2 months ago

This is type3 CPU only. If we use std::vector instead of malloc/free the memory leak goes away without extra effort. Othwewise we need to define an error handler in cpu finufft setpts for type 3 that make sure to free all the memory before returning. I'd rather do the de/macroize as it is less error prone.

This memory leak occurs only in case of errors so it is not super critical to fix as I expect the user to terminate the program if the nufft fails but we should fix it now that we spotted it.

ahbarnett commented 2 months ago

great, make a std::vector PR just for those and we'll bring to 2.3.1.

On Thu, Sep 5, 2024 at 12:33 PM Marco Barbone @.***> wrote:

This is type3 CPU only. If we use std::vector instead of malloc/free the memory leak goes away without extra effort. Othwewise we need to define an error handler in cpu finufft setpts for type 3 that make sure to free all the memory before returning. I'd rather do the de/macroize as it is less error prone.

This memory leak occurs only in case of errors so it is not super critical to fix as I expect the user to terminate the program if the nufft fails but we should fix it now that we spotted it.

— Reply to this email directly, view it on GitHub https://github.com/flatironinstitute/finufft/issues/547#issuecomment-2332178014, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACNZRSXKB2VHGK4XCJSTBVDZVCBUFAVCNFSM6AAAAABNVDDVNWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGMZSGE3TQMBRGQ . You are receiving this because you commented.Message ID: @.***>

-- *-------------------------------------------------------------------~^`^~._.~' |\ Alex Barnett Center for Computational Mathematics, Flatiron Institute | \ http://users.flatironinstitute.org/~ahb 646-876-5942

ahbarnett commented 2 months ago

Ok, we wait until de-macro-ize which will use std::vector.

mreineck commented 1 week ago

This should be fixed now, correct?

DiamonDinoia commented 1 week ago

Indeed.