Better handling of startup and shutdown of the debugger agent

Better handling of startup and shutdown of the debugger agent.

During bind and listen socket errors are now handled. If the listen socket is occoupied the agent will retry its bind operation until success or shutdown.

Added orderly shutdown of the debugger agent both with and without a client connected.

Committed: http://code.google.com/p/v8/source/detail?r=1552

[Erik Corry, 2009-03-19 13:17:13]

LGTM

http://codereview.chromium.org/50007/diff/1/5
File src/debug-agent.cc (right):

http://codereview.chromium.org/50007/diff/1/5#newcode52
Line 52: // If an error occoured wait a bit before retrying.
More comments?

http://codereview.chromium.org/50007/diff/1/5#newcode73
Line 73: terminate_now_->Wait(kOneSecondInMicros);
I think unless we have a concrete situation where we know it's good to wait then
we should probably not have a sleep here.

http://codereview.chromium.org/50007/diff/1/5#newcode79
Line 79: void DebuggerAgent::Shutdown() {
Is it possible to assert we have the lock here?

http://codereview.chromium.org/50007/diff/1/2
File test/cctest/test-debug.cc (right):

http://codereview.chromium.org/50007/diff/1/2#newcode3829
Line 3829: const int port = 5858;
You already used port 5858 in this file.  In the interest of being able to
parallelize the tests reliably you should use a different number.

[Søren Thygesen Gjesse, 2009-03-19 21:05:48]

http://codereview.chromium.org/50007/diff/1/5
File src/debug-agent.cc (right):

http://codereview.chromium.org/50007/diff/1/5#newcode52
Line 52: // If an error occoured wait a bit before retrying.
On 2009/03/19 13:17:13, Erik Corry wrote:
> More comments?

Done.

http://codereview.chromium.org/50007/diff/1/5#newcode73
Line 73: terminate_now_->Wait(kOneSecondInMicros);
On 2009/03/19 13:17:13, Erik Corry wrote:
> I think unless we have a concrete situation where we know it's good to wait
then
> we should probably not have a sleep here.

Agree, removed the sleep.

http://codereview.chromium.org/50007/diff/1/5#newcode79
Line 79: void DebuggerAgent::Shutdown() {
On 2009/03/19 13:17:13, Erik Corry wrote:
> Is it possible to assert we have the lock here?

We don't need the lock here as we are not looking as session_. CloseSession will
take the lock when terminating the session.

Added a Join() after server_->Shutdown() to make sure that the listener thread
is fully terminated before moving on. Otherwise we could get a new session
created just after CloseSession().

http://codereview.chromium.org/50007/diff/1/2
File test/cctest/test-debug.cc (right):

http://codereview.chromium.org/50007/diff/1/2#newcode3829
Line 3829: const int port = 5858;
On 2009/03/19 13:17:13, Erik Corry wrote:
> You already used port 5858 in this file.  In the interest of being able to
> parallelize the tests reliably you should use a different number.

Good point. Used different ports for different tests, and added a comment in the
tests about this.