tsdjs / tsd

Check TypeScript type definitions
MIT License
2.36k stars 68 forks source link

Memory crash since `@tsd/typescript@5.2.2` #214

Closed ehmicky closed 3 months ago

ehmicky commented 3 months ago

The following crashes:

$ tsd -f one.ts -t two.ts

<--- Last few GCs --->

[29461:0x69653a0]    23377 ms: Scavenge 4051.4 (4127.9) -> 4048.2 (4129.1) MB, 8.96 / 0.00 ms  (average mu = 0.987, current mu = 0.978) allocation failure; 
[29461:0x69653a0]    23395 ms: Scavenge 4052.7 (4129.1) -> 4049.4 (4132.1) MB, 9.50 / 0.00 ms  (average mu = 0.987, current mu = 0.978) allocation failure; 
[29461:0x69653a0]    23513 ms: Scavenge 4055.8 (4132.1) -> 4051.0 (4149.6) MB, 108.22 / 0.00 ms  (average mu = 0.987, current mu = 0.978) allocation failure; 

<--- JS stacktrace --->

FATAL ERROR: Reached heap limit Allocation failed - JavaScript heap out of memory
----- Native stack trace -----

 1: 0xcd8bd6 node::OOMErrorHandler(char const*, v8::OOMDetails const&) [tsd]
 2: 0x10aed20 v8::Utils::ReportOOMFailure(v8::internal::Isolate*, char const*, v8::OOMDetails const&) [tsd]
 3: 0x10af007 v8::internal::V8::FatalProcessOutOfMemory(v8::internal::Isolate*, char const*, v8::OOMDetails const&) [tsd]
 4: 0x12cdfe5  [tsd]
 5: 0x12e4b08  [tsd]
 6: 0x12bc00e v8::internal::HeapAllocator::AllocateRawWithLightRetrySlowPath(int, v8::internal::AllocationType, v8::internal::AllocationOrigin, v8::internal::AllocationAlignment) [tsd]
 7: 0x12bd2f4 v8::internal::HeapAllocator::AllocateRawWithRetryOrFailSlowPath(int, v8::internal::AllocationType, v8::internal::AllocationOrigin, v8::internal::AllocationAlignment) [tsd]
 8: 0x129afb5 v8::internal::Factory::NewFillerObject(int, v8::internal::AllocationAlignment, v8::internal::AllocationType, v8::internal::AllocationOrigin) [tsd]
 9: 0x16cb57c v8::internal::Runtime_AllocateInYoungGeneration(int, unsigned long*, v8::internal::Isolate*) [tsd]
10: 0x1b5b3f6  [tsd]
Aborted (core dumped)
// one.ts
import 'tsd';
import type {A, B} from './two';

const b: B = {} as A;
// two.ts
export type A = typeof a;

declare const a: B;

export type B<U = 0> = {s: C<0, U>};

type C<T, U> = U extends 0 | {t: 0}
    ? U extends [0] ? E<D<T, [U]>> : 0
    : 0;

type D<T, U> = U extends 0 | Array<0>
    ? T extends keyof U
        ? U[T] extends 2 ? U[T] : 0
        : 0
    : 0;

type E<U> = U extends 0 | {t: 0} ? U : 0;

System: Ubuntu 23.10, Node 21.7.2, latest tsd (0.31.0). I could also reproduce this bug on a GitHub action running on Windows, macOS and Linux. So this does not appear to be OS-related, nor machine-related. Node 16.17.0 crashes too, so this does not appear to be Node version-related either.

However, it does appear the above bug might be related to the 5.2.2 release of @tsd/typescript, which was shipped with tsd@0.29.0.

At first, it might look this is not an issue with tsd but with TypeScript. However:

If the above types do not make much sense, that's because they are a reduction of the original types (which did make sense). Although not quite sensical, those types are quite simple and not circular, so they should not cause a memory crash. I could not reduce them any further.

This bug is currently preventing Execa from upgrading tsd. Namely, upgrading tsd will make memory crash. The above is a reduction of that problem.

This is a very strange bug because very minor changes fail to reproduce it. This includes:

The memory crash happens after tsd runs for 10+ seconds. That's quite slow for such small types, so it seems like tsd is getting stuck in some loop until it crashes with OOM.

mrazauskas commented 3 months ago

I can reproduce directly with typescript (i.e. without installing tsd, @tsd/typescript and skipping the import from tsd). Bisecting points to https://github.com/microsoft/TypeScript/pull/51367 as the first bad commit. Does this make any sense?

ehmicky commented 3 months ago

This is interesting. When I run the above with tsc and do not import tsd, this does not crash for me. Also, it runs very fast, while tsd runs for 10+ seconds before crashing.

Also, I am curious how the above TypeScript commit would have an impact on the above. It only adds type definitions for Array.with(), Array.toSorted(), Array.toReversed(), Array.toSpliced(), but the example above is not using any of them. :thinking:

mrazauskas commented 3 months ago

Could you point to concrete types which are causing the problem, please? I always struggle to understand abstract A, B, C stuff. Easier to play with tools, but not so easy to understand the intent behind these types.

Do I get it right that your provided type D is doing something with the keys of an Array? Hm.. Not sure why this would be needed.. If that's the case, note that the PR I was referencing is adding more keys to the Array object. That could explain the OOM.

ehmicky commented 3 months ago

I would like to provide with a more meaningful example. However, the original types are quite complex. I spent several hours reducing them to the minimal example above, but it started to get nonsensical at some point.

That being said, while understanding the original intent would be helpful for a feature request, this is quite clearly a bug. Regardless of the original intent, the example above should not crash (even if the types do not make sense).

Now, I have been doing some additional research, and it appears that the problem is actually not related to tsd but to TypeScript. What seems to happen is: since TypeScript 5, some specific combinations of extends + Tuples seem to create big union types internally. Sometimes, this creates the following error:

TS2590: Expression produces a union type that is too complex

Other times, it crashes with OOM. In my case, the reason it fails only when using tsd seems to be that importing tsd requires some memory, which makes it go beyond the memory crash threshold.

I managed to reproduce the bug with only TypeScript, without tsd. Although it looks different, it seems to be the same underlying problem. I have opened https://github.com/microsoft/TypeScript/issues/58268 to track this. The example I used there is a little more sensical.

Thanks for looking into this! :+1: