Skip to content

Avoid retaining parameter tables for invalid content types - #309

Merged
ringabout merged 1 commit into
planety:develfrom
puffball1567:fix/content-type-memory-retention
Aug 19, 2026
Merged

ringabout merged 1 commit into
planety:develfrom
puffball1567:fix/content-type-memory-retention

Conversation

@puffball1567

Copy link
Copy Markdown
Contributor

Summary

  • Initialize the Content-Type parameter table only after validating the media type.
  • Preserve the existing ValueError behavior for empty and malformed media types.
  • Add empty Content-Type coverage to the existing parser tests.

Why

Prologue calls parseContentType("") for requests without a Content-Type header. The parser previously allocated the parameter table before validating the media type and raising ValueError.

In a persistent Prologue server, those allocations remained live during server operation and grew linearly with the request count. Valgrind reported no definitely-lost blocks at process exit, so this is runtime retention rather than an unreachable shutdown leak. It can still exhaust memory in a long-running server.

Massif attributed about 13.27 MiB of a 13.6 MiB live heap after approximately 5,200 requests to the table initialized in parseContentType. With this change, the measured peak was about 0.33 MiB.

In non-Valgrind runs with one concurrent persistent Joubako client:

  • Before: about 108 MiB growth after approximately 26,000 requests.
  • After: 48-56 KiB growth after approximately 37,000-39,000 requests.

Validation

  • Full Testament suite.
  • Existing Content-Type and multipart tests.
  • Additional valid, malformed, quoted, and multipart input checks.
  • ARC and ORC, with threads both enabled and disabled.
  • Valgrind Massif and Memcheck.

This addresses a reproducible contributor to the memory growth reported in #305. The original 700-concurrency reproducer may still have additional contributing factors.

@ringabout

Copy link
Copy Markdown
Member

Thank you! The leak is probably related to nim-lang/Nim#26094

@puffball1567

Copy link
Copy Markdown
Contributor Author

Thanks, that issue matches the behavior I observed very closely. This change should also provide a small defensive workaround for existing Nim releases while #26100 is being integrated. Thank you for working on the compiler-side fix!

@ringabout
ringabout merged commit 01edc25 into planety:devel Aug 19, 2026
2 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.

2 participants