Fix RaithData memory leaks

[MatthewMckee4]

Fix for #270.

Changes:

• updated type hints so raith_data is optional and its attributes are immutable
• Made FlexPath.raith_data in c++ optional<RaithData>
• moved raith to_gds functionality to the RaithData struct
• (accidentally) reformatted .pyi file (I can change this back if this is an issue)
• add tests for RaithData
• update tests for FlexPath.rath_data

[MatthewMckee4]

@heitzmann do you know what has causes this error? I am developing on Ubuntu 24.04 so was unaware of such an issue with macos and windows

[gnawhleinad]

From macos-latest run in https://github.com/heitzmann/gdstk/actions/runs/10512919194/job/29141203848?pr=271,

/Applications/Xcode_15.4.app/Contents/Developer/usr/bin/g++  -I/Users/runner/work/gdstk/gdstk/include -I/Users/runner/work/gdstk/gdstk/external -I/opt/homebrew/include -Wall -Wextra -Wshadow -Wvla -Wformat -Wno-missing-field-initializers -Wno-missing-braces -Wno-cast-function-type -Wno-unused-parameter -mmacosx-version-min=10.9 -O3 -DNDEBUG -O3 -DNDEBUG -std=c++11 -arch arm64 -isysroot /Applications/Xcode_15.4.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX14.5.sdk -fPIC -MD -MT src/CMakeFiles/gdstk.dir/flexpath.cpp.o -MF src/CMakeFiles/gdstk.dir/flexpath.cpp.o.d -o src/CMakeFiles/gdstk.dir/flexpath.cpp.o -c /Users/runner/work/gdstk/gdstk/src/flexpath.cpp
In file included from /Users/runner/work/gdstk/gdstk/src/flexpath.cpp:20:
/Users/runner/work/gdstk/gdstk/include/gdstk/flexpath.hpp:78:10: error: no template named 'optional' in namespace 'std'
    std::optional<RaithData> raith_data;
    ~~~~~^
1 error generated.

Or, very specifically, the -std=c++11.

<optional> was added in c++17 via https://en.cppreference.com/w/cpp/header/optional.

I suspect the Windows failure is similar.

[heitzmann]

@MatthewMckee4 That's correct, we have to switch to C++ 17 to use optional. From your changes, I believe the leak comes from the copy_from function. I patched it in https://github.com/heitzmann/gdstk/commit/69fbde19683f1e2dde84f2591f00e68c974a4d89 just to do a quick release because of other changes. You can try that version to see if it works, or make sure we can compile correctly in C++17 and continue this PR.

[MatthewMckee4]

in RaithData::copy_from the code is as follows

    ...
    if (base_cell_name) free_allocation(base_cell_name);
    base_cell_name = NULL;
    if (raith_data.base_cell_name) base_cell_name = copy_string(raith_data.base_cell_name, NULL);

I think it should just be

    ...    
    base_cell_name = NULL;
    if (raith_data.base_cell_name) base_cell_name = copy_string(raith_data.base_cell_name, NULL);

When the free_allocation call is there there's a memory leak:

free(): invalid pointer
Aborted (core dumped)

[heitzmann]

The problem is not the free call. It's wherever the flexpath was created without clearing this member. I found one instance in the python interface and already patched it in 0.9.55. You have to trace back where this variable was set with dirty memory

On Sun, Sep 1, 2024, 17:32 Matthew Mckee @.***> wrote:

in RaithData::copy_from the code is as follows

...
if (base_cell_name) free_allocation(base_cell_name);
base_cell_name = NULL;
if (raith_data.base_cell_name) base_cell_name = copy_string(raith_data.base_cell_name, NULL);

I think it should just be

...
base_cell_name = NULL;
if (raith_data.base_cell_name) base_cell_name = copy_string(raith_data.base_cell_name, NULL);

When the free_allocation call is there there's a memory leak:

free(): invalid pointer Aborted (core dumped)

— Reply to this email directly, view it on GitHub https://github.com/heitzmann/gdstk/pull/271#issuecomment-2323488177, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABNJ2E4QNBN7AW7FA3V4SVDZUN2VPAVCNFSM6AAAAABM6TUO3GVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGMRTGQ4DQMJXG4 . You are receiving this because you were mentioned.Message ID: @.***>

[MatthewMckee4]

Sorry for not getting back to you sooner, i believe this is fixed so will close this issues