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 invalid pointer to urlpath argument #47

Merged
merged 2 commits into from
Aug 21, 2022
Merged

Fix invalid pointer to urlpath argument #47

merged 2 commits into from
Aug 21, 2022

Conversation

cgohlke
Copy link
Contributor

@cgohlke cgohlke commented Aug 20, 2022

Fixes segfaults during tests on Windows in "development mode".

The C pointer storage.urlpath was pointing to the value of a Python bytes instance, which got deleted while the pointer was still in use.

I noticed that the return values of some C function calls are not checked for errors and c-blosc does not check some pointer arguments for NULL...

@FrancescAlted
Copy link
Member

Looks good! Thanks!

@FrancescAlted FrancescAlted merged commit a8bd9a7 into Blosc:main Aug 21, 2022
@cgohlke
Copy link
Contributor Author

cgohlke commented Aug 21, 2022

Does this fix the problems on aarch64 and musl platforms?

@FrancescAlted
Copy link
Member

Well, it is getting better. Now, I am getting only an Error: The operation was canceled, so definitely your patch helped.

Now, I am guessing that the job for building wheels for arm64 is being cancelled because it is taking too much time. Maybe splitting this job in two (say one for glibc and the other for musl), should help.

@cgohlke
Copy link
Contributor Author

cgohlke commented Aug 22, 2022

Do numpy and psutil provide wheels for or even support musllinux_aarch64? Maybe those packages are failing to install or build until CI times out?

@FrancescAlted
Copy link
Member

FrancescAlted commented Aug 24, 2022

Do numpy and psutil provide wheels for or even support musllinux_aarch64? Maybe those packages are failing to install or build until CI times out?

That's a good point. Apparently the numpy team is not building musllinux_aarch64 yet, and what is probably happening here is that CI is trying to build one from scratch.

Unfortunately, somehow the default is to build wheels with musllinux support (I suppose this is related with the incremental deployment of them via https://peps.python.org/pep-0656/). This, together with the lack of a native aarch64 platform in GH Actions, makes this very slow, and hence, prone to be cancelled due to timeout. So I think for now it is better to disable the wheels for aarch64 completely. When NumPy would have native wheels for aarch64 and muslinux this can be tackled again.

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.

2 participants