https://codereview.chromium.org/1226143003/diff/1/src/d8.cc
File src/d8.cc (right):

https://codereview.chromium.org/1226143003/diff/1/src/d8.cc#newcode1703
src/d8.cc:1703: case STOPPED:
On 2015/07/10 at 10:43:17, jarin wrote:
Why do we want to retry when STOPPED?

Actually, I am even confused why we are trying to go through the
TERMINATING state in WaitForThread. Why (instead of calling Terminate)
don't you CAS RUNNING->JOINING?

It would be helpful if you could describe what was wrong with the
original state machine.

You're right, this is too complicated. I refactored this into a much
simpler change.

Basically, the problem was that the pthread object would leak if
Worker.terminate() was never called. Really all that needed to be done
was call base::Thread::Join(), but that crashes if you call it twice. I
wanted to catch that programmer error, but adding it to the state_
variable made it too complex.

https://codereview.chromium.org/1226143003/diff/1/src/d8.cc#newcode1708
src/d8.cc:1708: case JOINING:
On 2015/07/10 at 10:43:17, jarin wrote:
Is not this only called when the shell finishes, so there should not
be anyone else joining?

True, this was just caution to prevent multiple joiners (which crashes).
I've refactored this into a much simpler change.

https://codereview.chromium.org/1226143003/diff/1/src/d8.cc#newcode2187
src/d8.cc:2187: char* p = new char[length];
On 2015/07/10 at 10:43:17, jarin wrote:
How about using std::vector<char> here, instead of the naked pointer?

Done (though I was not sure that std containers were allowed in this
file, it seems like they were explicitly avoided).

https://codereview.chromium.org/1226143003/diff/1/src/d8.h
File src/d8.h (right):

https://codereview.chromium.org/1226143003/diff/1/src/d8.h#newcode261
src/d8.h:261: enum State { IDLE, RUNNING, TERMINATING, STOPPED, JOINING,
JOINED };
On 2015/07/10 at 10:43:17, jarin wrote:
Could we have some descriptions of what the states mean?

Done.

https://codereview.chromium.org/1226143003/

--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
--- You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
For more options, visit https://groups.google.com/d/optout.

Reply via email to