Skip to content

Commit ec7b33e

Browse files
authored
[Turbopack] fix require.resolve self hanging (#69793)
### What? evaluation require.resolve tries to resolve and process a module during module analytics. Since analytics is part of processing a module this can create a call cycle when `require.resolve("./self")` is used. This happens in some npm packages and caused Turbopack to hange. This PR fixes that problem by only resolving the module and not processing it when using `require.resolve`. In addition to that it adds a panic when a call cycle occurs. The next time we mess it up and create a call cycle this will crash Turbopack instead of hanging, which makes it much easier to find the problem. fixes PACK-3227
1 parent a9bfc64 commit ec7b33e

File tree

6 files changed

+156
-24
lines changed

6 files changed

+156
-24
lines changed

turbopack/crates/turbo-tasks-memory/src/aggregation/notify_new_follower.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,13 @@ impl<C: AggregationContext> PreparedInternalOperation<C> for PreparedNotifyNewFo
189189
let follower = ctx.node(&follower_id);
190190
let follower_aggregation_number = follower.aggregation_number();
191191
if follower_aggregation_number == upper_aggregation_number {
192+
if upper_id == follower_id {
193+
panic!(
194+
"Cycle in call graph (A function calls itself \
195+
recursively with the same arguments. This will never \
196+
finish and would hang indefinitely.)"
197+
);
198+
}
192199
increase_aggregation_number_internal(
193200
ctx,
194201
balance_queue,

turbopack/crates/turbopack-core/src/resolve/mod.rs

Lines changed: 97 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -2752,10 +2752,6 @@ async fn resolve_package_internal_with_imports_field(
27522752
.await
27532753
}
27542754

2755-
async fn is_unresolveable(result: Vc<ModuleResolveResult>) -> Result<bool> {
2756-
Ok(*result.resolve().await?.is_unresolveable().await?)
2757-
}
2758-
27592755
pub async fn handle_resolve_error(
27602756
result: Vc<ModuleResolveResult>,
27612757
reference_type: Value<ReferenceType>,
@@ -2765,41 +2761,124 @@ pub async fn handle_resolve_error(
27652761
severity: Vc<IssueSeverity>,
27662762
source: Option<Vc<IssueSource>>,
27672763
) -> Result<Vc<ModuleResolveResult>> {
2764+
async fn is_unresolveable(result: Vc<ModuleResolveResult>) -> Result<bool> {
2765+
Ok(*result.resolve().await?.is_unresolveable().await?)
2766+
}
27682767
Ok(match is_unresolveable(result).await {
27692768
Ok(unresolveable) => {
27702769
if unresolveable {
2771-
ResolvingIssue {
2770+
emit_unresolveable_issue(
27722771
severity,
2773-
file_path: origin_path,
2774-
request_type: format!("{} request", reference_type.into_value()),
2772+
origin_path,
2773+
reference_type,
27752774
request,
27762775
resolve_options,
2777-
error_message: None,
27782776
source,
2779-
}
2780-
.cell()
2781-
.emit();
2777+
);
27822778
}
27832779

27842780
result
27852781
}
27862782
Err(err) => {
2787-
ResolvingIssue {
2783+
emit_resolve_error_issue(
27882784
severity,
2789-
file_path: origin_path,
2790-
request_type: format!("{} request", reference_type.into_value()),
2785+
origin_path,
2786+
reference_type,
27912787
request,
27922788
resolve_options,
2793-
error_message: Some(format!("{}", PrettyPrintError(&err))),
2789+
err,
27942790
source,
2795-
}
2796-
.cell()
2797-
.emit();
2791+
);
27982792
ModuleResolveResult::unresolveable().cell()
27992793
}
28002794
})
28012795
}
28022796

2797+
pub async fn handle_resolve_source_error(
2798+
result: Vc<ResolveResult>,
2799+
reference_type: Value<ReferenceType>,
2800+
origin_path: Vc<FileSystemPath>,
2801+
request: Vc<Request>,
2802+
resolve_options: Vc<ResolveOptions>,
2803+
severity: Vc<IssueSeverity>,
2804+
source: Option<Vc<IssueSource>>,
2805+
) -> Result<Vc<ResolveResult>> {
2806+
async fn is_unresolveable(result: Vc<ResolveResult>) -> Result<bool> {
2807+
Ok(*result.resolve().await?.is_unresolveable().await?)
2808+
}
2809+
Ok(match is_unresolveable(result).await {
2810+
Ok(unresolveable) => {
2811+
if unresolveable {
2812+
emit_unresolveable_issue(
2813+
severity,
2814+
origin_path,
2815+
reference_type,
2816+
request,
2817+
resolve_options,
2818+
source,
2819+
);
2820+
}
2821+
2822+
result
2823+
}
2824+
Err(err) => {
2825+
emit_resolve_error_issue(
2826+
severity,
2827+
origin_path,
2828+
reference_type,
2829+
request,
2830+
resolve_options,
2831+
err,
2832+
source,
2833+
);
2834+
ResolveResult::unresolveable().cell()
2835+
}
2836+
})
2837+
}
2838+
2839+
fn emit_resolve_error_issue(
2840+
severity: Vc<IssueSeverity>,
2841+
origin_path: Vc<FileSystemPath>,
2842+
reference_type: Value<ReferenceType>,
2843+
request: Vc<Request>,
2844+
resolve_options: Vc<ResolveOptions>,
2845+
err: anyhow::Error,
2846+
source: Option<Vc<IssueSource>>,
2847+
) {
2848+
ResolvingIssue {
2849+
severity,
2850+
file_path: origin_path,
2851+
request_type: format!("{} request", reference_type.into_value()),
2852+
request,
2853+
resolve_options,
2854+
error_message: Some(format!("{}", PrettyPrintError(&err))),
2855+
source,
2856+
}
2857+
.cell()
2858+
.emit();
2859+
}
2860+
2861+
fn emit_unresolveable_issue(
2862+
severity: Vc<IssueSeverity>,
2863+
origin_path: Vc<FileSystemPath>,
2864+
reference_type: Value<ReferenceType>,
2865+
request: Vc<Request>,
2866+
resolve_options: Vc<ResolveOptions>,
2867+
source: Option<Vc<IssueSource>>,
2868+
) {
2869+
ResolvingIssue {
2870+
severity,
2871+
file_path: origin_path,
2872+
request_type: format!("{} request", reference_type.into_value()),
2873+
request,
2874+
resolve_options,
2875+
error_message: None,
2876+
source,
2877+
}
2878+
.cell()
2879+
.emit();
2880+
}
2881+
28032882
// TODO this should become a TaskInput instead of a Vc
28042883
/// ModulePart represents a part of a module.
28052884
///

turbopack/crates/turbopack-ecmascript/src/references/mod.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ use turbopack_core::{
7272
source_map::{GenerateSourceMap, OptionSourceMap, SourceMap},
7373
};
7474
use turbopack_resolve::{
75-
ecmascript::{apply_cjs_specific_options, cjs_resolve},
75+
ecmascript::{apply_cjs_specific_options, cjs_resolve_source},
7676
typescript::tsconfig,
7777
};
7878
use turbopack_swc_utils::emitter::IssueEmitter;
@@ -2383,14 +2383,14 @@ async fn require_resolve_visitor(
23832383
Ok(if args.len() == 1 {
23842384
let pat = js_value_to_pattern(&args[0]);
23852385
let request = Request::parse(Value::new(pat.clone()));
2386-
let resolved = cjs_resolve(origin, request, None, IssueSeverity::Warning.cell())
2386+
let resolved = cjs_resolve_source(origin, request, None, IssueSeverity::Warning.cell())
23872387
.resolve()
23882388
.await?;
23892389
let mut values = resolved
2390-
.primary_modules()
2390+
.primary_sources()
23912391
.await?
23922392
.iter()
2393-
.map(|&module| async move { require_resolve(module.ident().path()).await })
2393+
.map(|&source| async move { require_resolve(source.ident().path()).await })
23942394
.try_join()
23952395
.await?;
23962396

turbopack/crates/turbopack-resolve/src/ecmascript.rs

Lines changed: 33 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,14 +4,14 @@ use turbopack_core::{
44
issue::{IssueSeverity, IssueSource},
55
reference_type::{CommonJsReferenceSubType, EcmaScriptModulesReferenceSubType, ReferenceType},
66
resolve::{
7-
handle_resolve_error,
7+
handle_resolve_error, handle_resolve_source_error,
88
options::{
99
ConditionValue, ResolutionConditions, ResolveInPackage, ResolveIntoPackage,
1010
ResolveOptions,
1111
},
1212
origin::{ResolveOrigin, ResolveOriginExt},
1313
parse::Request,
14-
ModuleResolveResult,
14+
resolve, ModuleResolveResult, ResolveResult,
1515
},
1616
};
1717
/// Retrieves the [ResolutionConditions] of both the "into" package (allowing a
@@ -103,6 +103,37 @@ pub async fn cjs_resolve(
103103
specific_resolve(origin, request, options, ty, issue_severity, issue_source).await
104104
}
105105

106+
#[turbo_tasks::function]
107+
pub async fn cjs_resolve_source(
108+
origin: Vc<Box<dyn ResolveOrigin>>,
109+
request: Vc<Request>,
110+
issue_source: Option<Vc<IssueSource>>,
111+
issue_severity: Vc<IssueSeverity>,
112+
) -> Result<Vc<ResolveResult>> {
113+
// TODO pass CommonJsReferenceSubType
114+
let ty = Value::new(ReferenceType::CommonJs(CommonJsReferenceSubType::Undefined));
115+
let options = apply_cjs_specific_options(origin.resolve_options(ty.clone()))
116+
.resolve()
117+
.await?;
118+
let result = resolve(
119+
origin.origin_path().parent().resolve().await?,
120+
ty.clone(),
121+
request,
122+
options,
123+
);
124+
125+
handle_resolve_source_error(
126+
result,
127+
ty,
128+
origin.origin_path(),
129+
request,
130+
options,
131+
issue_severity,
132+
issue_source,
133+
)
134+
.await
135+
}
136+
106137
async fn specific_resolve(
107138
origin: Vc<Box<dyn ResolveOrigin>>,
108139
request: Vc<Request>,

turbopack/crates/turbopack-tests/tests/execution/turbopack/resolving/require-resolve/input/index.js

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,3 +9,15 @@ it('should support require.resolve with extensions', () => {
99
'[project]/turbopack/crates/turbopack-tests/tests/execution/turbopack/resolving/require-resolve/input/resolved.js [test] (ecmascript)'
1010
)
1111
})
12+
13+
it('should support require.resolve on the current module', () => {
14+
expect(require.resolve('./index.js')).toBe(
15+
'[project]/turbopack/crates/turbopack-tests/tests/execution/turbopack/resolving/require-resolve/input/index.js [test] (ecmascript)'
16+
)
17+
// Check if static evaluation is not hanging
18+
if (require.resolve('./index.js') == "") {
19+
throw new Error("no no");
20+
}
21+
})
22+
23+
export {};
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
{
2+
"treeShakingMode": "reexports-only"
3+
}

0 commit comments

Comments
 (0)