Use OpenThread instead of DuplicateHandle in Win32 profiler implementation.

0 views
Skip to first unread message

mikhail...@gmail.com

unread,
Mar 25, 2009, 4:52:46 AM3/25/09
to sgj...@chromium.org, v8-...@googlegroups.com
Reviewers: Søren Gjesse,

Description:
Use OpenThread instead of DuplicateHandle in Win32 profiler
implementation.

OpenThread doesn't fail in Chrome sandbox, while DuplicateHandle does.

Please review this at http://codereview.chromium.org/49028

Affected files:
M src/platform-win32.cc


Index: src/platform-win32.cc
diff --git a/src/platform-win32.cc b/src/platform-win32.cc
index
f5d0913581311796912c0d449d2941ed4126fee3..8628c9648bb08e49645e2312856ebf45bccd14ae
100644
--- a/src/platform-win32.cc
+++ b/src/platform-win32.cc
@@ -1820,10 +1820,12 @@ void Sampler::Start() {
// going to profile. We need to duplicate the handle because we are
// going to use it in the sampler thread. using GetThreadHandle() will
// not work in this case.
- BOOL ok = DuplicateHandle(GetCurrentProcess(), GetCurrentThread(),
- GetCurrentProcess(),
&data_->profiled_thread_,
- THREAD_GET_CONTEXT | THREAD_SUSPEND_RESUME |
- THREAD_QUERY_INFORMATION, FALSE, 0);
+ data_->profiled_thread_ = OpenThread(THREAD_GET_CONTEXT |
+ THREAD_SUSPEND_RESUME |
+ THREAD_QUERY_INFORMATION,
+ FALSE,
+ GetCurrentThreadId());
+ BOOL ok = data_->profiled_thread_ != NULL;
if (!ok) return;
}



sgj...@chromium.org

unread,
Mar 25, 2009, 5:18:00 AM3/25/09
to mikhail...@gmail.com, v8-...@googlegroups.com
LGTM

This is great!


http://codereview.chromium.org/49028/diff/1/2
File src/platform-win32.cc (right):

http://codereview.chromium.org/49028/diff/1/2#newcode1818
Line 1818: if (IsProfiling()) {
I think the comment needs to be changed a bit as still says "duplicate
the handle". It could say "open an additional handle" and maybe mention
while DuplicateHandle is not used.

http://codereview.chromium.org/49028

mikhail...@gmail.com

unread,
Mar 25, 2009, 5:34:32 AM3/25/09
to sgj...@chromium.org, v8-...@googlegroups.com

http://codereview.chromium.org/49028/diff/1/2
File src/platform-win32.cc (right):

http://codereview.chromium.org/49028/diff/1/2#newcode1818
Line 1818: if (IsProfiling()) {
On 2009/03/25 09:18:00, Søren Gjesse wrote:
> I think the comment needs to be changed a bit as still says "duplicate
the
> handle". It could say "open an additional handle" and maybe mention
while
> DuplicateHandle is not used.

Done.

http://codereview.chromium.org/49028
Reply all
Reply to author
Forward
0 new messages