Skip to content

Commit 3059b49

Browse files
stepanchegfacebook-github-bot
authored andcommitted
Move check_starlark_stack_size into a separate file
Summary: It is clean well isolated piece of code, and `buck2_interpreter_for_build/src/interpreter/dice_calculation_delegate.rs` is too big already (and I need to make it bigger). Reviewed By: JakobDegen Differential Revision: D58788774 fbshipit-source-id: 018d4824248db3510acdcf744e4e10e8854f1ef3
1 parent a5505ee commit 3059b49

File tree

3 files changed

+96
-76
lines changed

3 files changed

+96
-76
lines changed

app/buck2_interpreter_for_build/src/interpreter.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ pub mod build_context;
1212
pub(crate) mod bzl_eval_ctx;
1313
pub mod calculation;
1414
pub(crate) mod cell_info;
15+
pub(crate) mod check_starlark_stack_size;
1516
pub mod configuror;
1617
pub mod context;
1718
pub mod cycles;
Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
/*
2+
* Copyright (c) Meta Platforms, Inc. and affiliates.
3+
*
4+
* This source code is licensed under both the MIT license found in the
5+
* LICENSE-MIT file in the root directory of this source tree and the Apache
6+
* License, Version 2.0 found in the LICENSE-APACHE file in the root directory
7+
* of this source tree.
8+
*/
9+
10+
use allocative::Allocative;
11+
use async_trait::async_trait;
12+
use buck2_futures::cancellation::CancellationContext;
13+
use buck2_interpreter::dice::starlark_provider::with_starlark_eval_provider;
14+
use buck2_interpreter::error::BuckStarlarkError;
15+
use buck2_interpreter::starlark_profiler::profiler::StarlarkProfilerOpt;
16+
use dice::DiceComputations;
17+
use dice::Key;
18+
use indoc::indoc;
19+
use starlark::environment::Globals;
20+
use starlark::environment::Module;
21+
use starlark::syntax::AstModule;
22+
use starlark::syntax::Dialect;
23+
24+
#[derive(Debug, buck2_error::Error)]
25+
enum CheckStarlarkStackSizeError {
26+
#[error("Error checking starlark stack size")]
27+
CheckStarlarkStackSizeError,
28+
}
29+
30+
// In order to prevent non deterministic crashes
31+
// we intentionally set off a starlark stack overflow, to make
32+
// sure that starlark catches the overflow and reports an error
33+
// before the native stack overflows
34+
pub(crate) async fn check_starlark_stack_size(
35+
ctx: &mut DiceComputations<'_>,
36+
) -> anyhow::Result<()> {
37+
#[derive(Debug, derive_more::Display, Clone, Allocative, Eq, PartialEq, Hash)]
38+
struct StarlarkStackSizeChecker;
39+
40+
#[async_trait]
41+
impl Key for StarlarkStackSizeChecker {
42+
type Value = buck2_error::Result<()>;
43+
44+
async fn compute(
45+
&self,
46+
ctx: &mut DiceComputations,
47+
_cancellation: &CancellationContext,
48+
) -> Self::Value {
49+
with_starlark_eval_provider(
50+
ctx,
51+
&mut StarlarkProfilerOpt::disabled(),
52+
"Check starlark stack size".to_owned(),
53+
move |provider, _| {
54+
let env = Module::new();
55+
let (mut eval, _) = provider.make(&env)?;
56+
let content = indoc!(
57+
r#"
58+
def f():
59+
f()
60+
f()
61+
"#
62+
);
63+
let ast = AstModule::parse("x.star", content.to_owned(), &Dialect::Extended)
64+
.map_err(BuckStarlarkError::new)?;
65+
match eval.eval_module(ast, &Globals::standard()) {
66+
Err(e) if e.to_string().contains("Starlark call stack overflow") => Ok(()),
67+
Err(p) => Err(BuckStarlarkError::new(p).into()),
68+
Ok(_) => {
69+
Err(CheckStarlarkStackSizeError::CheckStarlarkStackSizeError.into())
70+
}
71+
}
72+
},
73+
)
74+
.await?;
75+
Ok(())
76+
}
77+
78+
fn equality(x: &Self::Value, y: &Self::Value) -> bool {
79+
match (x, y) {
80+
(Ok(x), Ok(y)) => x == y,
81+
_ => false,
82+
}
83+
}
84+
85+
fn validity(x: &Self::Value) -> bool {
86+
x.is_ok()
87+
}
88+
}
89+
90+
ctx.compute(&StarlarkStackSizeChecker)
91+
.await?
92+
.map_err(anyhow::Error::from)
93+
}

app/buck2_interpreter_for_build/src/interpreter/dice_calculation_delegate.rs

Lines changed: 2 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@ use buck2_events::dispatch::span;
2929
use buck2_events::dispatch::span_async;
3030
use buck2_futures::cancellation::CancellationContext;
3131
use buck2_interpreter::dice::starlark_provider::with_starlark_eval_provider;
32-
use buck2_interpreter::error::BuckStarlarkError;
3332
use buck2_interpreter::file_loader::LoadedModule;
3433
use buck2_interpreter::file_loader::ModuleDeps;
3534
use buck2_interpreter::import_paths::HasImportPaths;
@@ -46,15 +45,12 @@ use dice::DiceComputations;
4645
use dice::Key;
4746
use dupe::Dupe;
4847
use futures::FutureExt;
49-
use indoc::indoc;
5048
use starlark::codemap::FileSpan;
51-
use starlark::environment::Globals;
52-
use starlark::environment::Module;
5349
use starlark::syntax::AstModule;
54-
use starlark::syntax::Dialect;
5550

5651
use crate::interpreter::buckconfig::ConfigsOnDiceViewForStarlark;
5752
use crate::interpreter::cell_info::InterpreterCellInfo;
53+
use crate::interpreter::check_starlark_stack_size::check_starlark_stack_size;
5854
use crate::interpreter::cycles::LoadCycleDescriptor;
5955
use crate::interpreter::global_interpreter_state::HasGlobalInterpreterState;
6056
use crate::interpreter::interpreter_for_cell::InterpreterForCell;
@@ -68,8 +64,6 @@ enum DiceCalculationDelegateError {
6864
EvalBuildFileError(BuildFilePath),
6965
#[error("Error evaluating module: `{0}`")]
7066
EvalModuleError(String),
71-
#[error("Error checking starlark stack size")]
72-
CheckStarlarkStackSizeError,
7367
}
7468

7569
#[async_trait]
@@ -442,80 +436,12 @@ impl<'c, 'd: 'c> DiceCalculationDelegate<'c, 'd> {
442436
.await
443437
}
444438

445-
// In order to prevent non deterministic crashes
446-
// we intentionally set off a starlark stack overflow, to make
447-
// sure that starlark catches the overflow and reports an error
448-
// before the native stack overflows
449-
async fn check_starlark_stack_size(&mut self) -> anyhow::Result<()> {
450-
#[derive(Debug, Display, Clone, Allocative, Eq, PartialEq, Hash)]
451-
struct StarlarkStackSizeChecker;
452-
453-
#[async_trait]
454-
impl Key for StarlarkStackSizeChecker {
455-
type Value = buck2_error::Result<()>;
456-
457-
async fn compute(
458-
&self,
459-
ctx: &mut DiceComputations,
460-
_cancellation: &CancellationContext,
461-
) -> Self::Value {
462-
with_starlark_eval_provider(
463-
ctx,
464-
&mut StarlarkProfilerOpt::disabled(),
465-
"Check starlark stack size".to_owned(),
466-
move |provider, _| {
467-
let env = Module::new();
468-
let (mut eval, _) = provider.make(&env)?;
469-
let content = indoc!(
470-
r#"
471-
def f():
472-
f()
473-
f()
474-
"#
475-
);
476-
let ast =
477-
AstModule::parse("x.star", content.to_owned(), &Dialect::Extended)
478-
.map_err(BuckStarlarkError::new)?;
479-
match eval.eval_module(ast, &Globals::standard()) {
480-
Err(e) if e.to_string().contains("Starlark call stack overflow") => {
481-
Ok(())
482-
}
483-
Err(p) => Err(BuckStarlarkError::new(p).into()),
484-
Ok(_) => {
485-
Err(DiceCalculationDelegateError::CheckStarlarkStackSizeError
486-
.into())
487-
}
488-
}
489-
},
490-
)
491-
.await?;
492-
Ok(())
493-
}
494-
495-
fn equality(x: &Self::Value, y: &Self::Value) -> bool {
496-
match (x, y) {
497-
(Ok(x), Ok(y)) => x == y,
498-
_ => false,
499-
}
500-
}
501-
502-
fn validity(x: &Self::Value) -> bool {
503-
x.is_ok()
504-
}
505-
}
506-
507-
self.ctx
508-
.compute(&StarlarkStackSizeChecker)
509-
.await?
510-
.map_err(anyhow::Error::from)
511-
}
512-
513439
pub async fn eval_build_file(
514440
&mut self,
515441
package: PackageLabel,
516442
profiler_instrumentation: &mut StarlarkProfilerOpt<'_>,
517443
) -> buck2_error::Result<Arc<EvaluationResult>> {
518-
self.check_starlark_stack_size().await?;
444+
check_starlark_stack_size(self.ctx).await?;
519445

520446
let listing = self.resolve_package_listing(package.dupe()).await?;
521447

0 commit comments

Comments
 (0)