Bug 100950

Summary:

[Chromium-Android] Apache doesn't properly clean up ipc semaphores after a layout test run

Product:

WebKit

Reporter:

Peter Beverloo <peter>

Component:

Tools / Tests

Assignee:

Peter Beverloo <peter>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

abarth, dpranke, ojan, wangxianzhu, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

Bug Blocks:

100100

Attachments:

Description	Flags
Patch	none
Patch	none

Peter Beverloo

Reported 2012-11-01 05:54:24 PDT

This is causing more than 100 semaphores to be left on the system, eventually causing apache2 failing to start (and thus the layout test run to abort) with this error: [error] (28)No space left on device: Cannot create SSLMutex A workaround is executing this command, which fixes it for two days, but this is not sustainable: $ ipcs -s | grep chrome-bot | perl -e 'while (<STDIN>) { @a=split(/\s+/); print `ipcrm sem $a[1]`}' We should figure out how to make apache properly clean up the semaphores, as this is causing too much flakiness.

Attachments
Patch (8.12 KB, patch) 2012-11-01 09:01 PDT, Peter Beverloo	no flags	Details Formatted Diff Diff
Patch (8.58 KB, patch) 2012-11-01 09:57 PDT, Peter Beverloo	no flags	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Peter Beverloo

Comment 1 2012-11-01 07:43:43 PDT

Just writing down some of my findings. Android controls the HTTP server not through Port.start_http_server() and Port.stop_http_server(), like other ports, but instead does so in Port.setup_test_run() and Port.clean_up_test_run(). The comment mentions that we do this because the HTTP server is needed during the whole test-run. Looking at the test output, it seems like the Manager._clean_up_run() method is not being called for runs which fail with an exception. Therefore we cannot gracefully shut down the HTTP server. This would explain the ipc semaphores not being removed. It seems like {clean_up,setup}_test_run() is called once per execution of the test runner, whereas the {start,stop}_http_server() methods are called once per run. For example, if we have to retry failing tests, the server would be started again. While this may introduce some delay, the benefit of not changing behavior is that the HTTP server would be shut down in the finally clause of a try/catch block, making it much more likely that it can gracefully shutdown. I'm going to upload a patch to revert our behavior to be in-line with other ports. Xianzhu, since you wrote this code, could you maybe comment on whether there are other issues I'm forgetting here?

Peter Beverloo

Comment 2 2012-11-01 09:01:37 PDT

Created attachment 171865 [details] Patch

Xianzhu Wang

Comment 3 2012-11-01 09:13:11 PDT

(In reply to comment #1) > I'm going to upload a patch to revert our behavior to be in-line with other ports. Xianzhu, since you wrote this code, could you maybe comment on whether there are other issues I'm forgetting here? Your patch looks good to me. Thanks. Haven't seen other issue.

Peter Beverloo

Comment 4 2012-11-01 09:57:04 PDT

Created attachment 171880 [details] Patch

Dirk Pranke

Comment 5 2012-11-01 11:02:43 PDT

Comment on attachment 171880 [details] Patch seems reasonable.

WebKit Review Bot

Comment 6 2012-11-01 11:40:51 PDT

Comment on attachment 171880 [details] Patch Clearing flags on attachment: 171880 Committed r133200: <http://trac.webkit.org/changeset/133200>

WebKit Review Bot

Comment 7 2012-11-01 11:40:55 PDT

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.