Skip to content

Conversation

@suhasravi7
Copy link
Contributor

@suhasravi7 suhasravi7 commented Sep 24, 2025

Summary

Adds custom memory allocator support to the JOSE library, allowing applications to override default malloc/realloc/free functions for specialized memory management needs.

Changes

  • Added jose_set_alloc() and jose_get_alloc() API functions
  • Added comprehensive test suite (api_mem.c)
  • Updated build system integration

Use Cases

  • Memory debugging and tracking
  • Custom memory pools
  • Secure memory allocation
  • Cross-DLL memory management

Testing

meson test -C build api_mem

All tests pass, validating functionality across different usage scenarios.

@suhasravi7 suhasravi7 changed the title Feature/custom allocator tests Feature/custom allocator Sep 24, 2025
@suhasravi7 suhasravi7 changed the title Feature/custom allocator Feature: Custom Memory Allocator Sep 24, 2025
@sarroutbi
Copy link
Collaborator

The size_t type is not a built-in keyword in C; it's defined in standard library headers like <stdlib.h>. Please, fix include/jose/cfg.h to include the corresponding one

@suhasravi7
Copy link
Contributor Author

The size_t type is not a built-in keyword in C; it's defined in standard library headers like <stdlib.h>. Please, fix include/jose/cfg.h to include the corresponding one

Updated the PR

@sarroutbi
Copy link
Collaborator

Hello @suhasravi7 . Thanks for the patch.

I guess it is not as easy as I thought ... could you please merge next change to your branch and incorporate it to the PR to check if compilation errors are fixed?

@sarroutbi
Copy link
Collaborator

Hello @suhasravi7 : Could you please justify why this should be merged? We don't see a clear requirement to change memory allocators, and we find some security constraints on including it ... which architecture, software, product, ... justifies this change? CC @sergio-correia

@suhasravi7
Copy link
Contributor Author

Hello @suhasravi7 : Could you please justify why this should be merged? We don't see a clear requirement to change memory allocators, and we find some security constraints on including it ... which architecture, software, product, ... justifies this change? CC @sergio-correia

Thank you for raising these concerns @sarroutbi.

The motivation for introducing a custom memory allocator support in JOSE stems from our deployment environment, where we utilize an H2O proxy(https://github.com/h2o/h2o). H2O internally pre-allocates and manages memory blocks to optimize performance and reduce allocation overhead, especially under high concurrency. To align with this architecture and fully leverage these performance benefits, we require JOSE to support custom memory allocators.

By enabling JOSE to use externally managed allocators, we can maintain consistent memory management strategies across our stack. This approach offers measurable improvements in allocation speed and reduces fragmentation, compared to standard malloc/calloc routines. Additionally, it allows for tighter control over memory usage, which is critical in high-performance and resource-constrained environments.

Regarding security, the implementation ensures that allocator hooks are strictly opt-in and can be sandboxed or audited according to deployment requirements. The change is modular and does not impact default behavior unless explicitly configured.

@vikaspushkar
Copy link

vikaspushkar commented Sep 26, 2025

should the api jose_cfg_set_alloc enforce consistency in the passed parameters. either all should be NULL or Non-NULL. a combination of NULL and Non Null paramters could lead to mis management of the allocated memory here?

@suhasravi7
Copy link
Contributor Author

should the api jose_cfg_set_alloc enforce consistency in the passed parameters. either all should be NULL or Non-NULL. a combination of NULL and Non Null paramters could lead to mis management of the allocated memory here?

@vikaspushkar updated the pr to handle this case

@suhasravi7 suhasravi7 force-pushed the feature/custom-allocator-tests branch from 33598fb to 64f90d6 Compare September 26, 2025 07:58
@sarroutbi
Copy link
Collaborator

Hello @suhasravi7 : Could you please justify why this should be merged? We don't see a clear requirement to change memory allocators, and we find some security constraints on including it ... which architecture, software, product, ... justifies this change? CC @sergio-correia

Thank you for raising these concerns @sarroutbi.

The motivation for introducing a custom memory allocator support in JOSE stems from our deployment environment, where we utilize an H2O proxy(https://github.com/h2o/h2o). H2O internally pre-allocates and manages memory blocks to optimize performance and reduce allocation overhead, especially under high concurrency. To align with this architecture and fully leverage these performance benefits, we require JOSE to support custom memory allocators.

By enabling JOSE to use externally managed allocators, we can maintain consistent memory management strategies across our stack. This approach offers measurable improvements in allocation speed and reduces fragmentation, compared to standard malloc/calloc routines. Additionally, it allows for tighter control over memory usage, which is critical in high-performance and resource-constrained environments.

Regarding security, the implementation ensures that allocator hooks are strictly opt-in and can be sandboxed or audited according to deployment requirements. The change is modular and does not impact default behavior unless explicitly configured.

Thanks for clarifying. Changes LGTM. Let's wait for a double opinion regarding this and we can merge it

Copy link
Collaborator

@sarroutbi sarroutbi left a comment

Choose a reason for hiding this comment

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

LGTM

@suhasravi7 suhasravi7 force-pushed the feature/custom-allocator-tests branch from 64f90d6 to dde2b04 Compare September 26, 2025 09:24
Copy link
Member

@simo5 simo5 left a comment

Choose a reason for hiding this comment

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

Just a comment, but I think this code can be made more compact and efficient.

@suhasravi7 suhasravi7 force-pushed the feature/custom-allocator-tests branch 3 times, most recently from 5e19c34 to 31516cc Compare September 29, 2025 06:49
@sarroutbi sarroutbi self-requested a review September 29, 2025 13:20
@suhasravi7 suhasravi7 force-pushed the feature/custom-allocator-tests branch 2 times, most recently from b571eb4 to 3b326f5 Compare September 30, 2025 07:43
- Implement global function pointers for memory allocation
- Add jose_set_alloc(), jose_get_alloc(), jose_reset_alloc()
- Add comprehensive test suite in api_mem.c
- Remove cfg dependencies from io.c structs
- All tests passing (26/26)
@suhasravi7 suhasravi7 requested a review from simo5 September 30, 2025 07:47
Copy link
Member

@simo5 simo5 left a comment

Choose a reason for hiding this comment

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

LGTM

@sarroutbi sarroutbi merged commit b58fdb1 into latchset:master Sep 30, 2025
22 checks passed
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.

5 participants