Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(246)

Issue 319573006: Add sandbox support for process memory limits (Closed)

Created:
6 years, 6 months ago by jschuh
Modified:
6 years, 6 months ago
CC:
chromium-reviews, darin-cc_chromium.org, jam
Visibility:
Public.

Description

Add sandbox support for process memory limits BUG=335192 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=275666

Patch Set 1 #

Patch Set 2 : wip #

Patch Set 3 : ready for review #

Total comments: 8

Patch Set 4 : feedback #

Patch Set 5 : feedback #

Total comments: 2

Patch Set 6 : nit #

Patch Set 7 : build fix #

Unified diffs Side-by-side diffs Delta from patch set Stats (+100 lines, -25 lines) Patch
M content/common/sandbox_win.cc View 1 2 3 1 chunk +6 lines, -2 lines 0 comments Download
M sandbox/win/src/broker_services.cc View 1 2 3 4 5 1 chunk +7 lines, -0 lines 0 comments Download
M sandbox/win/src/job.h View 1 2 3 4 1 chunk +2 lines, -1 line 0 comments Download
M sandbox/win/src/job.cc View 1 2 3 2 chunks +7 lines, -6 lines 0 comments Download
M sandbox/win/src/job_unittest.cc View 1 2 3 4 5 6 6 chunks +15 lines, -14 lines 0 comments Download
M sandbox/win/src/restricted_token_utils.cc View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M sandbox/win/src/sandbox_policy.h View 1 2 3 1 chunk +5 lines, -0 lines 0 comments Download
M sandbox/win/src/sandbox_policy_base.h View 1 2 3 2 chunks +2 lines, -0 lines 0 comments Download
M sandbox/win/src/sandbox_policy_base.cc View 1 2 3 3 chunks +14 lines, -1 line 0 comments Download
M sandbox/win/src/sandbox_types.h View 1 1 chunk +1 line, -0 lines 0 comments Download
M sandbox/win/tests/validation_tests/commands.cc View 1 2 3 1 chunk +19 lines, -0 lines 0 comments Download
M sandbox/win/tests/validation_tests/suite.cc View 1 2 3 1 chunk +21 lines, -0 lines 0 comments Download

Messages

Total messages: 13 (0 generated)
jschuh
Win7 and below don't support notifications on job memory limits. Instead it just fails on ...
6 years, 6 months ago (2014-06-05 05:32:16 UTC) #1
jschuh
Also might be worth ananta@ taking a look, since he's apparently fought with these job ...
6 years, 6 months ago (2014-06-05 21:34:33 UTC) #2
cpu_(ooo_6.6-7.5)
https://codereview.chromium.org/319573006/diff/60001/sandbox/win/src/broker_services.cc File sandbox/win/src/broker_services.cc (right): https://codereview.chromium.org/319573006/diff/60001/sandbox/win/src/broker_services.cc#newcode251 sandbox/win/src/broker_services.cc:251: FreeResources(tracker, SBOX_FATAL_MEMORY_EXCEEDED); The code seems structured such that FreeResources() ...
6 years, 6 months ago (2014-06-06 19:44:05 UTC) #3
jschuh
https://codereview.chromium.org/319573006/diff/60001/sandbox/win/src/broker_services.cc File sandbox/win/src/broker_services.cc (right): https://codereview.chromium.org/319573006/diff/60001/sandbox/win/src/broker_services.cc#newcode251 sandbox/win/src/broker_services.cc:251: FreeResources(tracker, SBOX_FATAL_MEMORY_EXCEEDED); On 2014/06/06 19:44:05, cpu wrote: > The ...
6 years, 6 months ago (2014-06-06 20:13:14 UTC) #4
jschuh
ptal: all feedback actioned.
6 years, 6 months ago (2014-06-06 22:35:01 UTC) #5
cpu_(ooo_6.6-7.5)
lgtm
6 years, 6 months ago (2014-06-07 01:03:58 UTC) #6
ananta
lgtm https://codereview.chromium.org/319573006/diff/120001/sandbox/win/src/broker_services.cc File sandbox/win/src/broker_services.cc (right): https://codereview.chromium.org/319573006/diff/120001/sandbox/win/src/broker_services.cc#newcode282 sandbox/win/src/broker_services.cc:282: BOOL res = ::TerminateJobObject(tracker->job, Alignment seems off here.
6 years, 6 months ago (2014-06-07 01:09:18 UTC) #7
jschuh
https://codereview.chromium.org/319573006/diff/120001/sandbox/win/src/broker_services.cc File sandbox/win/src/broker_services.cc (right): https://codereview.chromium.org/319573006/diff/120001/sandbox/win/src/broker_services.cc#newcode282 sandbox/win/src/broker_services.cc:282: BOOL res = ::TerminateJobObject(tracker->job, On 2014/06/07 01:09:17, ananta wrote: ...
6 years, 6 months ago (2014-06-07 03:00:50 UTC) #8
jschuh
The CQ bit was checked by jschuh@chromium.org
6 years, 6 months ago (2014-06-07 03:00:56 UTC) #9
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jschuh@chromium.org/319573006/140001
6 years, 6 months ago (2014-06-07 03:02:12 UTC) #10
jschuh
The CQ bit was checked by jschuh@chromium.org
6 years, 6 months ago (2014-06-07 05:09:31 UTC) #11
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jschuh@chromium.org/319573006/160001
6 years, 6 months ago (2014-06-07 05:10:17 UTC) #12
commit-bot: I haz the power
6 years, 6 months ago (2014-06-07 09:05:47 UTC) #13
Message was sent while issue was closed.
Change committed as 275666

Powered by Google App Engine
This is Rietveld 408576698