I think it's a good solution if we don't need a general ThreadLocal implementation. I've ...
15 years, 9 months ago
(2010-03-26 07:59:22 UTC)
#1
I think it's a good solution if we don't need a general ThreadLocal
implementation. I've added a note about issues with a windows implementation
that was brought up by Vlad during the review of the original patch.
http://codereview.appspot.com/755042/diff/1/3
File include/gtest/internal/gtest-port.h (right):
http://codereview.appspot.com/755042/diff/1/3#newcode1090
include/gtest/internal/gtest-port.h:1090: // when the thread exits. Or, if the
ThreadLocal instance dies in
There are challenges with implementing destruction of thread locals on thread
exit on windows. Maybe we can add a comment here that on other implementations
destruction of thread locals might happen when the ThreadLocal instance is
destroyed.
http://codereview.appspot.com/755042/diff/1/2
File test/gtest-port_test.cc (right):
http://codereview.appspot.com/755042/diff/1/2#newcode990
test/gtest-port_test.cc:990: // should have been destroyed.
When we'll have a windows implementation this test should be pthread only.
Thanks for the comments, Miklos. I'll make the Windows-related changes when we decide to implement ...
15 years, 9 months ago
(2010-03-26 15:47:14 UTC)
#2
Thanks for the comments, Miklos. I'll make the Windows-related
changes when we decide to implement ThreadLocal on Windows, as the
whole design may change at that time, making many things moot.
Thanks,
On Fri, Mar 26, 2010 at 12:59 AM, <FazekasMiklos@gmail.com> wrote:
> I think it's a good solution if we don't need a general ThreadLocal
> implementation. I've added a note about issues with a windows
> implementation that was brought up by Vlad during the review of the
> original patch.
>
>
> http://codereview.appspot.com/755042/diff/1/3
> File include/gtest/internal/gtest-port.h (right):
>
> http://codereview.appspot.com/755042/diff/1/3#newcode1090
> include/gtest/internal/gtest-port.h:1090: // when the thread exits. Or,
> if the ThreadLocal instance dies in
> There are challenges with implementing destruction of thread locals on
> thread exit on windows. Maybe we can add a comment here that on other
> implementations destruction of thread locals might happen when the
> ThreadLocal instance is destroyed.
>
> http://codereview.appspot.com/755042/diff/1/2
> File test/gtest-port_test.cc (right):
>
> http://codereview.appspot.com/755042/diff/1/2#newcode990
> test/gtest-port_test.cc:990: // should have been destroyed.
> When we'll have a windows implementation this test should be pthread
> only.
>
> http://codereview.appspot.com/755042/show
>
--
Zhanyong
Issue 755042: Fixes leak in ThreadLocal.
Created 15 years, 9 months ago by wan
Modified 10 years, 10 months ago
Reviewers: fazekasmiklos_gmail.com
Base URL: http://googletest.googlecode.com/svn/trunk/
Comments: 2