Bug 100950 - [Chromium-Android] Apache doesn't properly clean up ipc semaphores after a layout test run
Summary: [Chromium-Android] Apache doesn't properly clean up ipc semaphores after a la...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Peter Beverloo
URL:
Keywords:
Depends on:
Blocks: 100100
  Show dependency treegraph
 
Reported: 2012-11-01 05:54 PDT by Peter Beverloo
Modified: 2012-11-01 11:40 PDT (History)
5 users (show)

See Also:


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

Note You need to log in before you can comment on or make changes to this bug.
Description Peter Beverloo 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.
Comment 1 Peter Beverloo 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?
Comment 2 Peter Beverloo 2012-11-01 09:01:37 PDT
Created attachment 171865 [details]
Patch
Comment 3 Xianzhu Wang 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.
Comment 4 Peter Beverloo 2012-11-01 09:57:04 PDT
Created attachment 171880 [details]
Patch
Comment 5 Dirk Pranke 2012-11-01 11:02:43 PDT
Comment on attachment 171880 [details]
Patch

seems reasonable.
Comment 6 WebKit Review Bot 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>
Comment 7 WebKit Review Bot 2012-11-01 11:40:55 PDT
All reviewed patches have been landed.  Closing bug.