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

libzstd: use custom memory functions #16028

Conversation

neiljohari
Copy link
Contributor

@neiljohari neiljohari commented Jan 17, 2025

Problem

When using automatic zstd decoding from libcurl, libcurl's custom allocators are not passed through to libzstd.

Solution

To configure libzstd's custom allocators, we must pass them through "advanced" APIs:

typedef void* (*ZSTD_allocFunction) (void* opaque, size_t size);
typedef void  (*ZSTD_freeFunction) (void* opaque, void* address);
typedef struct { ZSTD_allocFunction customAlloc; ZSTD_freeFunction customFree; void* opaque; } ZSTD_customMem;
static
#ifdef __GNUC__
__attribute__((__unused__))
#endif
ZSTD_customMem const ZSTD_defaultCMem = { NULL, NULL, NULL };  /**< this constant defers to stdlib's functions */
  These prototypes make it possible to pass your own allocation/free functions.
  ZSTD_customMem is provided at creation time, using ZSTD_create*_advanced() variants listed below.
  All allocation/free operations will be completed using these custom variants instead of regular  ones.

These APIs don't are not usable without static linking (as they're considered experimental and unstable, avoiding dynamic linking against a breaking change):

  Advanced experimental functions can be accessed using
  `#define ZSTD_STATIC_LINKING_ONLY` before including zstd.h.

  Advanced experimental APIs should never be used with a dynamically-linked
  library. They are not "stable"; their definitions or signatures may change in
  the future. Only static linking is allowed.

So for curl users who are using zstd, if they want to set custom allocators they should pass in a compiler flag ZSTD_STATIC_LINKING_ONLY, example:

target_compile_definitions(libcurl_static
        PRIVATE $<$<BOOL:${CURL_ZSTD}>:ZSTD_STATIC_LINKING_ONLY>
)

This function was introduced in v0.8.1 in 2016 here: facebook/zstd@be6180c (signature hasn't changed since then afaict).

@testclutch
Copy link

Analysis of PR #16028 at 5e629a7d:

Test http/test_07_upload.py::TestUpload::test_07_22_upload_parallel_fail[0-h3] failed, which has NOT been flaky recently, so there could be a real issue in this PR.

Generated by Testclutch

@neiljohari neiljohari marked this pull request as ready for review January 17, 2025 15:58
Copy link
Member

@bagder bagder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems fine, if even perhaps a bit... brittle to use such a non-public way. But since it is opt-in, I think it is fine to support.

@bagder bagder closed this in c807151 Jan 21, 2025
@neiljohari
Copy link
Contributor Author

Thanks! Appreciate the quick review 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants