Description
A corrupt file in the offline cache directory acts as a poison pill: the SDK tries to buffer the whole file into memory on every SentrySdk.Init, and the file is never discarded. On a large corrupt file this results in extreme memory usage and a very slow startup, on every single launch, until the user deletes the cache directory by hand.
Reported by @lgarczyn in #5507, where one of their designers' Macs crashed and came back extremely sluggish with hundreds of GB of memory/swap in use, traced to Sentry repeatedly trying to read a giant cache file.
Mechanism
Three things combine:
1. The envelope header read is unbounded.
StreamExtensions.ReadLineAsync reads 128-byte chunks into a MemoryStream until it finds a \n or reaches EOF. It has no length cap. Envelope.DeserializeAsync calls it as the very first thing it does, to read the envelope header.
An unclean shutdown can leave a cache file with no newline anywhere in it — on APFS, a file extended but whose data blocks were never flushed comes back NUL-filled. Reading a "header" out of such a file buffers the entire file into memory: a doubling MemoryStream working through 1 GB and 2 GB array allocations plus copies before it finally dies.
There is a second instance of the same class of bug in EnvelopeItem.DeserializePayloadAsync, which takes length straight from the item header and does (int)(payloadLength ?? stream.Length) before renting a buffer of that size. A corrupt-but-parseable header with a bogus length, or a large file with no length key at all, gives either an unchecked int overflow to a negative value or a very large positive allocation.
2. The discard path cannot catch the failure.
CachingTransport.InnerProcessCacheAsync catches only JsonException around the deserialize, and that catch is what deletes the file. What actually comes out of a runaway ReadLineAsync is OutOfMemoryException, or IOException("Stream was too long") once the buffer passes the 2 GB array limit. Neither is a JsonException, so the exception escapes to the worker's blanket catch (Exception), which logs, sleeps 500 ms and carries on — leaving the file in place.
The same gap applies to InvalidOperationException("Envelope header is malformed.") thrown by Envelope.DeserializeHeaderAsync, and to InvalidOperationException("Envelope item header is malformed.") in EnvelopeItem. Those are already reachable today with a file that contains valid JSON that isn't an object, and they also escape the discard.
3. The file comes back on every launch.
By the time it fails, the file has been moved to __processing, so within a single process it only detonates once. But MoveUnprocessedFilesBackToCache runs on every Initialize, which puts it straight back in the cache directory for the next run. With a non-zero InitCacheFlushTimeout (the default for several integrations) this happens while init is blocking, so it directly delays app startup.
There is no path out of this other than manually deleting the cache directory.
Aggravating factor: LogFailureWithDiscard calls ReadAllTextFromFile on the whole file to include its contents in the error log. So the one code path that does discard a bad file allocates the entire file as a string first.
Expected Result
A cache file that cannot be deserialized is discarded, with a bounded amount of memory and a bounded amount of work, and does not come back on the next launch.
Actual Result
The SDK buffers the entire file into memory, fails with an exception the discard path does not handle, leaves the file in the cache, and repeats on every launch.
Reproduction
Write a NUL-filled file with a .envelope extension into the process-specific cache directory (Path.Combine(CacheDirectoryPath, "Sentry", <dsn hash>)) and call SentrySdk.Init with CacheDirectoryPath set. The file is still there afterwards, and the same thing happens on the next init. Size it to taste for the memory symptom.
There is a test covering this in #5507.
Notes
#5507 addresses (1) for the header read and the aggravating factor. Worth deciding as part of that PR whether to also cover the payload-length path and the InvalidOperationException cases, or whether to handle the whole class by treating any non-cancellation failure out of Envelope.DeserializeAsync as "this file is unreadable, discard it".
Description
A corrupt file in the offline cache directory acts as a poison pill: the SDK tries to buffer the whole file into memory on every
SentrySdk.Init, and the file is never discarded. On a large corrupt file this results in extreme memory usage and a very slow startup, on every single launch, until the user deletes the cache directory by hand.Reported by @lgarczyn in #5507, where one of their designers' Macs crashed and came back extremely sluggish with hundreds of GB of memory/swap in use, traced to Sentry repeatedly trying to read a giant cache file.
Mechanism
Three things combine:
1. The envelope header read is unbounded.
StreamExtensions.ReadLineAsyncreads 128-byte chunks into aMemoryStreamuntil it finds a\nor reaches EOF. It has no length cap.Envelope.DeserializeAsynccalls it as the very first thing it does, to read the envelope header.An unclean shutdown can leave a cache file with no newline anywhere in it — on APFS, a file extended but whose data blocks were never flushed comes back NUL-filled. Reading a "header" out of such a file buffers the entire file into memory: a doubling
MemoryStreamworking through 1 GB and 2 GB array allocations plus copies before it finally dies.There is a second instance of the same class of bug in
EnvelopeItem.DeserializePayloadAsync, which takeslengthstraight from the item header and does(int)(payloadLength ?? stream.Length)before renting a buffer of that size. A corrupt-but-parseable header with a boguslength, or a large file with nolengthkey at all, gives either an unchecked int overflow to a negative value or a very large positive allocation.2. The discard path cannot catch the failure.
CachingTransport.InnerProcessCacheAsynccatches onlyJsonExceptionaround the deserialize, and that catch is what deletes the file. What actually comes out of a runawayReadLineAsyncisOutOfMemoryException, orIOException("Stream was too long")once the buffer passes the 2 GB array limit. Neither is aJsonException, so the exception escapes to the worker's blanketcatch (Exception), which logs, sleeps 500 ms and carries on — leaving the file in place.The same gap applies to
InvalidOperationException("Envelope header is malformed.")thrown byEnvelope.DeserializeHeaderAsync, and toInvalidOperationException("Envelope item header is malformed.")inEnvelopeItem. Those are already reachable today with a file that contains valid JSON that isn't an object, and they also escape the discard.3. The file comes back on every launch.
By the time it fails, the file has been moved to
__processing, so within a single process it only detonates once. ButMoveUnprocessedFilesBackToCacheruns on everyInitialize, which puts it straight back in the cache directory for the next run. With a non-zeroInitCacheFlushTimeout(the default for several integrations) this happens while init is blocking, so it directly delays app startup.There is no path out of this other than manually deleting the cache directory.
Aggravating factor:
LogFailureWithDiscardcallsReadAllTextFromFileon the whole file to include its contents in the error log. So the one code path that does discard a bad file allocates the entire file as a string first.Expected Result
A cache file that cannot be deserialized is discarded, with a bounded amount of memory and a bounded amount of work, and does not come back on the next launch.
Actual Result
The SDK buffers the entire file into memory, fails with an exception the discard path does not handle, leaves the file in the cache, and repeats on every launch.
Reproduction
Write a NUL-filled file with a
.envelopeextension into the process-specific cache directory (Path.Combine(CacheDirectoryPath, "Sentry", <dsn hash>)) and callSentrySdk.InitwithCacheDirectoryPathset. The file is still there afterwards, and the same thing happens on the next init. Size it to taste for the memory symptom.There is a test covering this in #5507.
Notes
#5507 addresses (1) for the header read and the aggravating factor. Worth deciding as part of that PR whether to also cover the payload-length path and the
InvalidOperationExceptioncases, or whether to handle the whole class by treating any non-cancellation failure out ofEnvelope.DeserializeAsyncas "this file is unreadable, discard it".