Bug 142145

Summary:

LayoutTestRelay: App environment variables not set for --guard-malloc or --leaks

Product:

WebKit

Reporter:

David Kilzer (:ddkilzer) <ddkilzer>

Component:

Tools / Tests

Assignee:

David Kilzer (:ddkilzer) <ddkilzer>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

ap, commit-queue, darin, dbates, ddkilzer, jake.nielsen.webkit, mrowe, msaboff, simon.fraser

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch v1	none

Description David Kilzer (:ddkilzer) 2015-03-01 07:43:51 PST

When running iOS Simulator tests with --guard-malloc or --leaks, LayoutTestRelay is not passing through the appropriate environment variables, so DumpRenderTree.app and WebKitTestRunner.app are not running with these environment variables set.

Comment 1 David Kilzer (:ddkilzer) 2015-03-01 07:48:14 PST

Created attachment 247630 [details]
Patch v1

Comment 2 Alexey Proskuryakov 2015-03-01 10:56:57 PST

Comment on attachment 247630 [details]
Patch v1

View in context: https://bugs.webkit.org/attachment.cgi?id=247630&action=review

> Tools/LayoutTestRelay/LayoutTestRelay/LTRelayController.m:211
> +            @"__XPC_DYLD_FRAMEWORK_PATH": productDirectory,

Adding __XPC_ variants of DYLD_FRAMEWORK_PATH and DYLD_LIBRARY_PATH doesn't seem like part of this fix. What does it fix?

Comment 3 WebKit Commit Bot 2015-03-01 10:57:22 PST

Comment on attachment 247630 [details]
Patch v1

Clearing flags on attachment: 247630

Committed r180864: <http://trac.webkit.org/changeset/180864>

Comment 4 WebKit Commit Bot 2015-03-01 10:57:25 PST

All reviewed patches have been landed.  Closing bug.

Comment 5 Darin Adler 2015-03-01 11:07:28 PST

Comment on attachment 247630 [details]
Patch v1

View in context: https://bugs.webkit.org/attachment.cgi?id=247630&action=review

> Tools/LayoutTestRelay/LayoutTestRelay/LTRelayController.m:225
> +        environmentVariables = [dictionary copy];

Is this tool compiled with ARC? If not, we need a [dictionary release].

Comment 6 David Kilzer (:ddkilzer) 2015-03-01 11:39:34 PST

(In reply to comment #2)
> Comment on attachment 247630 [details]
> Patch v1
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=247630&action=review
> 
> > Tools/LayoutTestRelay/LayoutTestRelay/LTRelayController.m:211
> > +            @"__XPC_DYLD_FRAMEWORK_PATH": productDirectory,
> 
> Adding __XPC_ variants of DYLD_FRAMEWORK_PATH and DYLD_LIBRARY_PATH doesn't
> seem like part of this fix. What does it fix?

It doesn't fix any known issue.  It makes the environment for any XPC-launched process consistent with its parent.  Is that bad?

Comment 7 David Kilzer (:ddkilzer) 2015-03-01 11:41:40 PST

(In reply to comment #5)
> Comment on attachment 247630 [details]
> Patch v1
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=247630&action=review
> 
> > Tools/LayoutTestRelay/LayoutTestRelay/LTRelayController.m:225
> > +        environmentVariables = [dictionary copy];
> 
> Is this tool compiled with ARC? If not, we need a [dictionary release].

Yes, it's compiled with ARC.

Comment 8 David Kilzer (:ddkilzer) 2015-03-01 11:49:56 PST

(In reply to comment #3)
> Comment on attachment 247630 [details]
> Patch v1
> 
> Clearing flags on attachment: 247630
> 
> Committed r180864: <http://trac.webkit.org/changeset/180864>

Fix think-o:  Committed r180866: <http://trac.webkit.org/changeset/180866>

Comment 9 Alexey Proskuryakov 2015-03-01 12:10:08 PST

> It doesn't fix any known issue.  It makes the environment for any XPC-launched process consistent with its parent.  Is that bad?

To me, it seems wrong to have code that does strange things with weird underscores and doesn't actually achieve anything. The functionality of __XPC environment variables is deeply suspect in general, and I expect that people will be looking at these lines in the future, trying to figure out if they can be removed or reimplemented.