Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sandbox2 wrapper #1536

Merged
merged 113 commits into from
Aug 31, 2023
Merged

Sandbox2 wrapper #1536

merged 113 commits into from
Aug 31, 2023

Conversation

lewish
Copy link
Collaborator

@lewish lewish commented Aug 29, 2023

Adds a Sandbox2 wrapper for compilation.

  • Change the way we communicate between child processes to use unix domain sockets
  • Add new webpacked worker bundle, and unbundled vm2 dependencies
  • Add Sandbox2 wrapper script for running the worker bundle inside node
  • Update tests to test both sandbox2 and non-sandbox2 mode

Disable tests for CLI (which is now broken in this branch), and integration tests that have a package dependency issue on this older branch.

Ekrekr and others added 30 commits January 7, 2021 14:31
* Bump protobufjs from 6.8.8 to 6.11.3

Bumps [protobufjs](https://github.com/protobufjs/protobuf.js) from 6.8.8 to 6.11.3.
- [Release notes](https://github.com/protobufjs/protobuf.js/releases)
- [Changelog](https://github.com/protobufjs/protobuf.js/blob/v6.11.3/CHANGELOG.md)
- [Commits](protobufjs/protobuf.js@6.8.8...v6.11.3)

---
updated-dependencies:
- dependency-name: protobufjs
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <support@github.com>

* Update protobufjs dep versions

* Upgrade to protobufjs v7.0.0, node to 16x

* Bump bazel to version 5.2.0

* Bump test CI to correct bazel version

* 3.5.0

* 3.5.0

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Lewis Hemens <lewishemens@google.com>
dependabot bot and others added 19 commits July 17, 2023 11:13
Bumps [protobufjs](https://github.com/protobufjs/protobuf.js) from 7.0.0 to 7.2.4.
- [Release notes](https://github.com/protobufjs/protobuf.js/releases)
- [Changelog](https://github.com/protobufjs/protobuf.js/blob/master/CHANGELOG.md)
- [Commits](protobufjs/protobuf.js@protobufjs-v7.0.0...protobufjs-v7.2.4)

---
updated-dependencies:
- dependency-name: protobufjs
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* Include multiline string literals when creating statements

* Bump version to 2.6.2
tests/api/examples.spec.ts Dismissed Show dismissed Hide dismissed
@lewish lewish requested a review from Ekrekr August 30, 2023 08:45
WORKSPACE Outdated Show resolved Hide resolved
api/commands/compile.ts Show resolved Hide resolved
sandbox/compile_executor.cc Outdated Show resolved Hide resolved
{
auto result = s2.AwaitResultWithTimeout(
absl::Seconds(TIMEOUT_SECS + FALLBACK_TIMEOUT_DELAY));
LOG(ERROR) << "sandbox failed to start: " << result->ToString();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are things logged to stderr readable from the Javascript? I'm slightly confused by the socket usage above.

Copy link
Collaborator Author

@lewish lewish Aug 30, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They are, but we don't read them. But this might be useful if we do run into errors, that they will be in our logs at least.
Basically, all we care about is the process return code for the sandbox. The logs are ignored, and all communication back to the parent parent process goes indirectly via the socket - the path to which we pass along as an argument.

Copy link
Contributor

@Ekrekr Ekrekr Aug 31, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair enough, if we need to add errors based off of the sandbox2 final result code we can do that later

sandbox/compile_executor.cc Show resolved Hide resolved
sandbox/worker/worker.ts Outdated Show resolved Hide resolved
scripts/cloudbuild/bazel_test Outdated Show resolved Hide resolved
@lewish lewish requested a review from Ekrekr August 30, 2023 13:12
@lewish lewish merged commit 112dad4 into main_v1 Aug 31, 2023
4 checks passed
@lewish lewish deleted the sandbox-api-lewis branch August 31, 2023 09:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet