-
-
Notifications
You must be signed in to change notification settings - Fork 201
Description
The sentry_envelope_write_to_path was always built on top of sentry_envelope_serialize purely for convenience, and has always had a comment suggesting it should stream directly to the output file:
sentry-native/src/sentry_envelope.c
Lines 476 to 482 in e8c7f15
| sentry_envelope_write_to_path( | |
| const sentry_envelope_t *envelope, const sentry_path_t *path) | |
| { | |
| // TODO: This currently builds the whole buffer in-memory. | |
| // It would be nice to actually stream this to a file. | |
| size_t buf_len = 0; | |
| char *buf = sentry_envelope_serialize(envelope, &buf_len); |
One customer is now running into the problem that the SDK writes invalid envelopes to disk on OOM, most likely because it is unable to allocate memory itself.
Such envelopes may have item headers (with correct length), but no item contents, most likely because writing the contents would grow the buffer which fails, so the serialization code then just silently ignores that error and continues writing the next item header.
In another case, the envelope contains attachments, but no event. Most likely because the code to builds envelopes currently reads attachments into memory first, before constructing the event datastructure, which also increases memory pressure.
Likely the most reasonable effort change would be to resolve that TODO which has existed ever since the code was written to truely stream envelope writes to disk, which should halve the memory requirement for the whole operation.
Second, but probably more effort, and also with an observable side-effect would be to load attachments on-demand when serializing, vs when building the envelope in-memory.