Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(269)

Issue 6862005: Initialize ThreadLocalTop. (Closed)

Created:
9 years, 8 months ago by Vitaly Repeshko
Modified:
9 years, 7 months ago
CC:
v8-dev
Visibility:
Public.

Description

Initialize ThreadLocalTop. ThreadLocalTop used to be static and was zero initialized by the linker. With isolates we have to give it a constructor. BUG=http://crbug.com/79393 Committed: http://code.google.com/p/v8/source/detail?r=7635

Patch Set 1 #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+29 lines, -9 lines) Patch
M src/isolate.h View 2 chunks +6 lines, -0 lines 0 comments Download
M src/top.cc View 3 chunks +23 lines, -9 lines 2 comments Download

Messages

Total messages: 4 (0 generated)
Vitaly Repeshko
9 years, 8 months ago (2011-04-15 02:07:22 UTC) #1
Søren Thygesen Gjesse
LGTM http://codereview.chromium.org/6862005/diff/1/src/top.cc File src/top.cc (right): http://codereview.chromium.org/6862005/diff/1/src/top.cc#newcode77 src/top.cc:77: InitializeInternal(); Do you need to call InitializeInternal() here ...
9 years, 8 months ago (2011-04-15 07:23:57 UTC) #2
Mads Ager (chromium)
Thanks for fixing this VItaly. Please merge to the 3.2 branch when landed.
9 years, 8 months ago (2011-04-15 08:03:27 UTC) #3
Vitaly Repeshko
9 years, 8 months ago (2011-04-15 20:47:56 UTC) #4
http://codereview.chromium.org/6862005/diff/1/src/top.cc
File src/top.cc (right):

http://codereview.chromium.org/6862005/diff/1/src/top.cc#newcode77
src/top.cc:77: InitializeInternal();
On 2011/04/15 07:23:57, Søren Gjesse wrote:
> Do you need to call InitializeInternal() here as well as in the constructor?

Yes, because Inititalize is called more than once to reset state.

Powered by Google App Engine
This is Rietveld 408576698