Change free to free_aligned in gmtlib_free_vector_ptr #8755
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
💖 Thanks for opening this pull request! 💖 Please make sure you read our Contributing Guide and abide by our Code of Conduct. A few things to keep in mind:
|
Member
|
Hi, Could you please provide an example where a crash occurred? Operating System? |
Contributor
Author
|
This is using Debian via WSL 2 on Windows 11 Pro 23H2. (the mkl version of lapack comes from installing pytorch) import pygmt
data = pygmt.datasets.load_sample_data(name="japan_quakes")
region = [
data.longitude.min() - 1,
data.longitude.max() + 1,
data.latitude.min() - 1,
data.latitude.max() + 1,
]
fig = pygmt.Figure()
fig.basemap(region=region, projection="M15c", frame=True)
fig.coast(land="black", water="skyblue")
fig.plot(x=data.longitude, y=data.latitude, style="c0.3c", fill="white", pen="black")
fig.show()It errors at the |
Member
|
Hi @gsitcia, |
Member
|
I don’t understand. You mentioned a crash in C because of some memory was allocated by FFTW but now this example has no call to any fftw function. And please, restrict here to plain GMT commands, not pygmt. Your PR cannot be accepted as is because it changes a too general function used in many other situations. We need to fully reproduce the crash in or to see what’s the best fix. |
Contributor
Author
|
Crash log: or
Both of these are on a Centos 7 system at work, because my own laptop doesn't print the backtrace (and I don't know how to make it do that) Here's a more explicit example using ctypes import ctypes as ctp
import numpy
# l = [0]*1_000_000
lib = ctp.CDLL('/home/intern/Alexander/micromamba/envs/test_3/lib/libgmt.so')
create_session = lib['GMT_Create_Session']
create_session.argtypes=[ctp.c_char_p, ctp.c_uint, ctp.c_uint, ctp.c_void_p]
create_session.restype=ctp.c_void_p
destroy_session = lib['GMT_Destroy_Session']
destroy_session.argtypes=[ctp.c_void_p]
create_data = lib['GMT_Create_Data']
create_data.argtypes=[ctp.c_void_p, ctp.c_uint, ctp.c_uint, ctp.c_uint, ctp.POINTER(ctp.c_uint64),
ctp.POINTER(ctp.c_double), ctp.POINTER(ctp.c_double), ctp.c_uint, ctp.c_int, ctp.c_void_p]
API = create_session(b'test', 2, 0, None)
create_data(API, 6, 1, 0, (ctp.c_ulong * 3)(1, 1, 9), None, None, 0, 2, None)
destroy_session(API)
# import time
# time.sleep(1)
It doesn't crash when I comment out the The fftw stuff I was referring to is this line in Line 551 in 1ed5e1b which I believe is called (via Line 9102 in 1ed5e1b on the other hand, Line 9212 in 1ed5e1b which always calls Line 612 in 1ed5e1b |
Member
|
Thank you for your effort but I still can't find a way to reproduce this from a pure CLI call. And am I interpreting wrong or the crash comes from python itself? |
Member
|
I believe that your ctypes example is equivalent to this Julia calls, where all goes fine (but I do have |
Contributor
Author
|
I found a way to reproduce the error with a gmt command. This should be run in the same conda environment as before. The issue for python is that when numpy is loaded before libgmt, numpy loads libcblas.so.3 which exports a version of That said, I feel my patch is correct just on the basis that memory allocated with |
Member
I agree with this, but the problem with your patch is that always frees with Is this restricted to Linux? I don't use Conda nor Python to further test this, @seisman ? |
Member
|
I can reproduce it under Linux, but not macOS. |
Member
|
OK, I think I have to revise my statement above. Did a more deep search and in fact |
joa-quim
approved these changes
Jul 3, 2025
|
🎉🎉🎉 Congrats on merging your first pull request and welcome to the team! 🎉🎉🎉 We hope that this was a good experience for you. Let us know if there is any way that the contributing process could be improved. |
Member
|
OK, I'm merging this. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Change call of
gmt_M_freetogmt_M_free_alignedingmtlib_free_vector_ptrto matchgmt_M_memory_alignedingmt_create_vector.At least for me, it was segfaulting because it would allocate with fftw and then try to deallocate with free.