Can you please add the --enable-logging flag and then fish for the generated chrome_debug.log and share it here. It should be in the UserDataDir location which defaults to %PROFILE%\AppData\Local\Google\Chrome\User Data\ if not changed by policy or command line flag.

When I try this in my testing Citrix env which admittedly is Win Srv 2012 Chrome 54 runs fine but I can see a lot of Out-Of-Memmory errors in the log which I will investigate in the meanwhile but they don't seem to cause the browser to get killed.

I am currently setting up a Win Srv 2008 R2 env to test there but will appreciate if you can upload some logs in the meantime. 

NOTE: The usual warning to check for personal info in the logs before uploading it applies again!

Hello,

I can confirm this issue with RDS, where Chrome is launched as a RemoteApp in the same fashion as reported here. Two RDS servers are involved, both running Windows Server 2008 r2. I appended the --enable-logging flag but the resulting chrome_debug.log file is empty for the user account I launched Chrome with.

Using --no-sandbox gets around the issue. Launching Chrome from the system console at the desktop normally does not yield the issue, either.

Adding some of the owners of the win sandbox in case they have some guesses what might have changed since 53 that would cause this.

We were able to reproduce this on win2008r2 with Chrome as a remote app in TS.

More to come...

I am guessing we may need to re spin M54 after a fix.

Which version of Citrix are you running? We are aware of incompatibilities between older, unsupported, versions of Citrix and the Chrome sandbox.

I can confirm that the issue occurs with XenApp 6.5 Hotfix Rollup 6.  Chrome Version 54.0.2840.71 m.

Our workaround has been to use --no-sandbox.

+jbauman author of the suspect CL (he is ooo till 11/6 though)

I could reproduce the issue on master and also could identify the problem being this CL: https://codereview.chromium.org/2316333003 - Fix sandbox::PolicyBase leak.

It causes the main browser process to crash with a access violation in SharedMemIPCServer::ThreadPingEventReady when the browser is running outside of a job.

I am currently looking if a small fix is possible or whether we should consider reverting the CL first and looking for a better solution later.

After analyzing the CL and debugging the well behaving codepath vs. the failing one I figured out the issue. When there is a job for the process which is always the case except when Chrome is running in a RDP session on a pre 2012 Windows server (Citrix or TS doesn't matter) we will create a JobTracker object for it which has a scoped_refptr for the PolicyBase oject. This happens  in BrokerServicesBase::SpawnTarget which in turn gets put in a tracker_list_ in the BrockerServiceBase object. This gives one more reference to the policy object and keeps it alive. However if the job is not created then the JobTracker isn't either and therefore the policy object ref counter goes to 0 and the object is destroyed.

My suggested course of action is to revert this CL on master and then on the release branch to avoid introducing new bugs. Reverting the CL is possible even on trunk with little manual work (kdiff3 managed to resolve all merge conflicts on ToT for me automatically after git revert).

Then we can figure on master if we want to have the JobTracker object always created even if there is no job or fix the lifetime of the PolicyBase object in another way.

M54 TPMs and sandbox owners what do you think?

I would prefer reverting the patch in trunk as well and in release branch. Would like Anantha/ Richard to make the final call.

Matt: Will it be possible for your team to verify the fix in case of a revert?

I think we should revert on trunk and merge the CL.

we should also add tests for this. I thought we already tested --no-sandbox-job but perhaps we are missing some coverage.

Will, would you mind reverting the patch?I am not able to contact pastarmovj@ at this moment.

Hey, I am here. I will do the revert now.

We can verify the fix and 100% agree that we should add tests for this :)

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2bfe4d60b683f05eb7fcb96f9a65d8279923f13f

commit 2bfe4d60b683f05eb7fcb96f9a65d8279923f13f
Author: pastarmovj <pastarmovj@chromium.org>
Date: Wed Oct 26 21:29:14 2016

Revert "Fix sandbox::PolicyBase leak"

This reverts commit b6a4ff86c730756a73d63cc882ef818fb7818a53
because it breaks Chrome when running as a published app on
Windows Terminal Services or Citrix XenApp.

Original issue's description:
> Fix sandbox::PolicyBase leak
>
> StartSandboxedProcess was missing a Release call, so this was always
> leaking. Use scoped_refptr instead to fix that and prevent it from
> happening in the future.
> TBR=piman@chromium.org
> CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng
>
> Committed: https://crrev.com/b6a4ff86c730756a73d63cc882ef818fb7818a53
> Cr-Commit-Position: refs/heads/master@{#417313}

BUG= 659026 
TBR=wfh@chromium.org,cpu@chromium.org,jbauman@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng

Review-Url: https://codereview.chromium.org/2456573002
Cr-Commit-Position: refs/heads/master@{#427808}

[modify] https://crrev.com/2bfe4d60b683f05eb7fcb96f9a65d8279923f13f/content/common/sandbox_init_win.cc
[modify] https://crrev.com/2bfe4d60b683f05eb7fcb96f9a65d8279923f13f/content/common/sandbox_win.cc
[modify] https://crrev.com/2bfe4d60b683f05eb7fcb96f9a65d8279923f13f/sandbox/win/sandbox_poc/main_ui_window.cc
[modify] https://crrev.com/2bfe4d60b683f05eb7fcb96f9a65d8279923f13f/sandbox/win/src/broker_services.cc
[modify] https://crrev.com/2bfe4d60b683f05eb7fcb96f9a65d8279923f13f/sandbox/win/src/broker_services.h
[modify] https://crrev.com/2bfe4d60b683f05eb7fcb96f9a65d8279923f13f/sandbox/win/src/policy_target_test.cc
[modify] https://crrev.com/2bfe4d60b683f05eb7fcb96f9a65d8279923f13f/sandbox/win/src/sandbox.h
[modify] https://crrev.com/2bfe4d60b683f05eb7fcb96f9a65d8279923f13f/sandbox/win/tests/common/controller.cc
[modify] https://crrev.com/2bfe4d60b683f05eb7fcb96f9a65d8279923f13f/sandbox/win/tests/common/controller.h

Requesting merge for https://codereview.chromium.org/2456573002

Approved for merging into M54

Hi Richard,

Can you share when the next 54 build will be available? I know many customers are stuck waiting, so I'd rather set their expectations now if possible.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/94ee730b26ee227a4696b0f28d958f2a8fd43e5b

commit 94ee730b26ee227a4696b0f28d958f2a8fd43e5b
Author: Julian Pastarmov <pastarmovj@chromium.org>
Date: Wed Oct 26 21:45:50 2016

Revert "Fix sandbox::PolicyBase leak"

This reverts commit b6a4ff86c730756a73d63cc882ef818fb7818a53
because it breaks Chrome when running as a published app on
Windows Terminal Services or Citrix XenApp.

Original issue's description:
> Fix sandbox::PolicyBase leak
>
> StartSandboxedProcess was missing a Release call, so this was always
> leaking. Use scoped_refptr instead to fix that and prevent it from
> happening in the future.
> TBR=piman@chromium.org
> CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng
>
> Committed: https://crrev.com/b6a4ff86c730756a73d63cc882ef818fb7818a53
> Cr-Commit-Position: refs/heads/master@{#417313}

BUG= 659026 
TBR=wfh@chromium.org,cpu@chromium.org,jbauman@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng

Review-Url: https://codereview.chromium.org/2456573002
Cr-Commit-Position: refs/heads/master@{#427808}
(cherry picked from commit 2bfe4d60b683f05eb7fcb96f9a65d8279923f13f)

Review URL: https://codereview.chromium.org/2453063003 .

Cr-Commit-Position: refs/branch-heads/2840@{#780}
Cr-Branched-From: 1ae106dbab4bddd85132d5b75c670794311f4c57-refs/heads/master@{#414607}

[modify] https://crrev.com/94ee730b26ee227a4696b0f28d958f2a8fd43e5b/content/common/sandbox_init_win.cc
[modify] https://crrev.com/94ee730b26ee227a4696b0f28d958f2a8fd43e5b/content/common/sandbox_win.cc
[modify] https://crrev.com/94ee730b26ee227a4696b0f28d958f2a8fd43e5b/sandbox/win/sandbox_poc/main_ui_window.cc
[modify] https://crrev.com/94ee730b26ee227a4696b0f28d958f2a8fd43e5b/sandbox/win/src/broker_services.cc
[modify] https://crrev.com/94ee730b26ee227a4696b0f28d958f2a8fd43e5b/sandbox/win/src/broker_services.h
[modify] https://crrev.com/94ee730b26ee227a4696b0f28d958f2a8fd43e5b/sandbox/win/src/policy_target_test.cc
[modify] https://crrev.com/94ee730b26ee227a4696b0f28d958f2a8fd43e5b/sandbox/win/src/sandbox.h
[modify] https://crrev.com/94ee730b26ee227a4696b0f28d958f2a8fd43e5b/sandbox/win/tests/common/controller.cc
[modify] https://crrev.com/94ee730b26ee227a4696b0f28d958f2a8fd43e5b/sandbox/win/tests/common/controller.h

Marking fixed now that the CL has landed on master and 54. I will mark it verified after testing it tomorrow. We can then think about merging to 55 as well. 

Thanks everyone for helping resolve this so quickly! :)

PS. A post-mortem on this one is certainly needed too.

Re: #19 we're testing a build tonight with the fix, I'd estimate tomorrow 10/27 or friday 10/28 for the new stable to be available.

Does this require a merge to M55? If yes, please request a merge to M55 ASAP. Thank you.

I just verified the fix on 54.0.2480.80 and it works as expected!

Therefore requesting merge to 55 as well.

Deleted: chrome_published.png 1.2 MB

	chrome_published.png 1.2 MB View Download

Approving merge to M55 branch 2883 based on comment #25. Please merge ASAP. Thank you.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/cfc155174ad04bc706b8b2a7a883583bdc219e9c

commit cfc155174ad04bc706b8b2a7a883583bdc219e9c
Author: Julian Pastarmov <pastarmovj@chromium.org>
Date: Thu Oct 27 16:07:08 2016

Revert "Fix sandbox::PolicyBase leak"

This reverts commit b6a4ff86c730756a73d63cc882ef818fb7818a53
because it breaks Chrome when running as a published app on
Windows Terminal Services or Citrix XenApp.

Original issue's description:
> Fix sandbox::PolicyBase leak
>
> StartSandboxedProcess was missing a Release call, so this was always
> leaking. Use scoped_refptr instead to fix that and prevent it from
> happening in the future.
> TBR=piman@chromium.org
> CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng
>
> Committed: https://crrev.com/b6a4ff86c730756a73d63cc882ef818fb7818a53
> Cr-Commit-Position: refs/heads/master@{#417313}

BUG= 659026 
TBR=wfh@chromium.org,cpu@chromium.org,jbauman@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng

Review-Url: https://codereview.chromium.org/2456573002
Cr-Commit-Position: refs/heads/master@{#427808}
(cherry picked from commit 2bfe4d60b683f05eb7fcb96f9a65d8279923f13f)

Review URL: https://codereview.chromium.org/2456063002 .

Cr-Commit-Position: refs/branch-heads/2883@{#334}
Cr-Branched-From: 614d31daee2f61b0180df403a8ad43f20b9f6dd7-refs/heads/master@{#423768}

[modify] https://crrev.com/cfc155174ad04bc706b8b2a7a883583bdc219e9c/content/common/sandbox_init_win.cc
[modify] https://crrev.com/cfc155174ad04bc706b8b2a7a883583bdc219e9c/content/common/sandbox_win.cc
[modify] https://crrev.com/cfc155174ad04bc706b8b2a7a883583bdc219e9c/sandbox/win/sandbox_poc/main_ui_window.cc
[modify] https://crrev.com/cfc155174ad04bc706b8b2a7a883583bdc219e9c/sandbox/win/src/broker_services.cc
[modify] https://crrev.com/cfc155174ad04bc706b8b2a7a883583bdc219e9c/sandbox/win/src/broker_services.h
[modify] https://crrev.com/cfc155174ad04bc706b8b2a7a883583bdc219e9c/sandbox/win/src/policy_target_test.cc
[modify] https://crrev.com/cfc155174ad04bc706b8b2a7a883583bdc219e9c/sandbox/win/src/sandbox.h
[modify] https://crrev.com/cfc155174ad04bc706b8b2a7a883583bdc219e9c/sandbox/win/tests/common/controller.cc
[modify] https://crrev.com/cfc155174ad04bc706b8b2a7a883583bdc219e9c/sandbox/win/tests/common/controller.h

Issue 656999 has been merged into this issue.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/cfc155174ad04bc706b8b2a7a883583bdc219e9c

commit cfc155174ad04bc706b8b2a7a883583bdc219e9c
Author: Julian Pastarmov <pastarmovj@chromium.org>
Date: Thu Oct 27 16:07:08 2016

Revert "Fix sandbox::PolicyBase leak"

This reverts commit b6a4ff86c730756a73d63cc882ef818fb7818a53
because it breaks Chrome when running as a published app on
Windows Terminal Services or Citrix XenApp.

Original issue's description:
> Fix sandbox::PolicyBase leak
>
> StartSandboxedProcess was missing a Release call, so this was always
> leaking. Use scoped_refptr instead to fix that and prevent it from
> happening in the future.
> TBR=piman@chromium.org
> CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng
>
> Committed: https://crrev.com/b6a4ff86c730756a73d63cc882ef818fb7818a53
> Cr-Commit-Position: refs/heads/master@{#417313}

BUG= 659026 
TBR=wfh@chromium.org,cpu@chromium.org,jbauman@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng

Review-Url: https://codereview.chromium.org/2456573002
Cr-Commit-Position: refs/heads/master@{#427808}
(cherry picked from commit 2bfe4d60b683f05eb7fcb96f9a65d8279923f13f)

Review URL: https://codereview.chromium.org/2456063002 .

Cr-Commit-Position: refs/branch-heads/2883@{#334}
Cr-Branched-From: 614d31daee2f61b0180df403a8ad43f20b9f6dd7-refs/heads/master@{#423768}

[modify] https://crrev.com/cfc155174ad04bc706b8b2a7a883583bdc219e9c/content/common/sandbox_init_win.cc
[modify] https://crrev.com/cfc155174ad04bc706b8b2a7a883583bdc219e9c/content/common/sandbox_win.cc
[modify] https://crrev.com/cfc155174ad04bc706b8b2a7a883583bdc219e9c/sandbox/win/sandbox_poc/main_ui_window.cc
[modify] https://crrev.com/cfc155174ad04bc706b8b2a7a883583bdc219e9c/sandbox/win/src/broker_services.cc
[modify] https://crrev.com/cfc155174ad04bc706b8b2a7a883583bdc219e9c/sandbox/win/src/broker_services.h
[modify] https://crrev.com/cfc155174ad04bc706b8b2a7a883583bdc219e9c/sandbox/win/src/policy_target_test.cc
[modify] https://crrev.com/cfc155174ad04bc706b8b2a7a883583bdc219e9c/sandbox/win/src/sandbox.h
[modify] https://crrev.com/cfc155174ad04bc706b8b2a7a883583bdc219e9c/sandbox/win/tests/common/controller.cc
[modify] https://crrev.com/cfc155174ad04bc706b8b2a7a883583bdc219e9c/sandbox/win/tests/common/controller.h

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/94ee730b26ee227a4696b0f28d958f2a8fd43e5b

commit 94ee730b26ee227a4696b0f28d958f2a8fd43e5b
Author: Julian Pastarmov <pastarmovj@chromium.org>
Date: Wed Oct 26 21:45:50 2016

Revert "Fix sandbox::PolicyBase leak"

This reverts commit b6a4ff86c730756a73d63cc882ef818fb7818a53
because it breaks Chrome when running as a published app on
Windows Terminal Services or Citrix XenApp.

Original issue's description:
> Fix sandbox::PolicyBase leak
>
> StartSandboxedProcess was missing a Release call, so this was always
> leaking. Use scoped_refptr instead to fix that and prevent it from
> happening in the future.
> TBR=piman@chromium.org
> CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng
>
> Committed: https://crrev.com/b6a4ff86c730756a73d63cc882ef818fb7818a53
> Cr-Commit-Position: refs/heads/master@{#417313}

BUG= 659026 
TBR=wfh@chromium.org,cpu@chromium.org,jbauman@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng

Review-Url: https://codereview.chromium.org/2456573002
Cr-Commit-Position: refs/heads/master@{#427808}
(cherry picked from commit 2bfe4d60b683f05eb7fcb96f9a65d8279923f13f)

Review URL: https://codereview.chromium.org/2453063003 .

Cr-Commit-Position: refs/branch-heads/2840@{#780}
Cr-Branched-From: 1ae106dbab4bddd85132d5b75c670794311f4c57-refs/heads/master@{#414607}

[modify] https://crrev.com/94ee730b26ee227a4696b0f28d958f2a8fd43e5b/content/common/sandbox_init_win.cc
[modify] https://crrev.com/94ee730b26ee227a4696b0f28d958f2a8fd43e5b/content/common/sandbox_win.cc
[modify] https://crrev.com/94ee730b26ee227a4696b0f28d958f2a8fd43e5b/sandbox/win/sandbox_poc/main_ui_window.cc
[modify] https://crrev.com/94ee730b26ee227a4696b0f28d958f2a8fd43e5b/sandbox/win/src/broker_services.cc
[modify] https://crrev.com/94ee730b26ee227a4696b0f28d958f2a8fd43e5b/sandbox/win/src/broker_services.h
[modify] https://crrev.com/94ee730b26ee227a4696b0f28d958f2a8fd43e5b/sandbox/win/src/policy_target_test.cc
[modify] https://crrev.com/94ee730b26ee227a4696b0f28d958f2a8fd43e5b/sandbox/win/src/sandbox.h
[modify] https://crrev.com/94ee730b26ee227a4696b0f28d958f2a8fd43e5b/sandbox/win/tests/common/controller.cc
[modify] https://crrev.com/94ee730b26ee227a4696b0f28d958f2a8fd43e5b/sandbox/win/tests/common/controller.h

 Issue 655645  has been merged into this issue.

Apparently our code in TargetProcess::~TargetProcess to deal with the case where the child process outlives the job object is also not working as intended.