code review 7197050: runtime: make LockOSThread nestable

runtime: make LockOSThread nestable

This change makes calling LockOSThread and UnlockOSThread safe
in library code, which is important when making syscalls that
return pointers to per-thread data. Without this change it is
unsafe to call UnlockOSThread in such cases, since if user
already called LockOSThread then UnlockOSThread would unlock
it unconditionally.

Fixes issue 3775.

[snaury, 2013-01-23 19:42:42]

Hello golang-dev@googlegroups.com,

I'd like you to review this change to
https://code.google.com/p/go/

[snaury, 2013-01-23 20:03:28]

On 2013/01/23 19:42:42, snaury wrote:
> Hello mailto:golang-dev@googlegroups.com,
> 
> I'd like you to review this change to
> https://code.google.com/p/go/

Ooops, now init() may unlock os thread, which shouldn't happen. Will fix
shortly.

[snaury, 2013-01-23 20:16:15]

Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com),

Please take another look.

[snaury, 2013-01-23 20:30:07]

Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com),

Please take another look.

[snaury, 2013-01-23 20:30:30]

Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com),

Please take another look.

[dave_cheney.net, 2013-01-23 20:39:01]

Could we say reenterent rather than nestable ?

On 24/01/2013, at 7:16, snaury@gmail.com wrote:

> Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com),
> 
> Please take another look.
> 
> 
> https://codereview.appspot.com/7197050/

[snaury, 2013-01-23 20:43:13]

On 2013/01/23 20:39:01, dfc wrote:
> Could we say reenterent rather than nestable ?

I'm not sure, according to wikipedia:

http://en.wikipedia.org/wiki/Reentrancy_(computing)

Reentrancy is about interruption and unfinished calls. And acconding to that
definition LockOSThread is not reentrant, since it cannot be interrupted and
re-entered. That's why I said nestable. :)

[remyoudompheng, 2013-01-23 21:12:30]

On 2013/1/23  <snaury@gmail.com> wrote:
> On 2013/01/23 20:39:01, dfc wrote:
>>
>> Could we say reenterent rather than nestable ?
>
>
> I'm not sure, according to wikipedia:
>
> http://en.wikipedia.org/wiki/Reentrancy_(computing)
>
> Reentrancy is about interruption and unfinished calls. And acconding to
> that definition LockOSThread is not reentrant, since it cannot be
> interrupted and re-entered. That's why I said nestable. :)
>
> https://codereview.appspot.com/7197050/

Maybe you could say this patch makes it a semaphore.

Rémy.

[rsc, 2013-01-23 21:50:05]

NOT LGTM sorry

I believe this is an API change and therefore not something we should do.
It is easy to simulate what you need by launching a new goroutine, having
it lock, and then waiting for it to finish. It's probably not even that
much more expensive.

Russ

[snaury, 2013-01-24 05:54:10]

On 2013/01/23 21:50:05, rsc wrote:
> NOT LGTM sorry
> 
> I believe this is an API change and therefore not something we should do.

OK, what about exporting runtime.LockedOSThread? It's already there, it's only
used for tests at the moment, but having would make this possible:

if !runtime.LockedOSThread() {
    runtime.LockOSThread()
    defer runtime.UnlockOSThread()
}

I think launching new goroutine just for locking (and then sending the result
back) is an overkill in this case.

[rsc, 2013-01-30 17:04:37]

I believe we've found a better solution in the Windows case.

In general runtime.LockOSThread is so ugly it should not exist. I don't
want to add more ugliness. Making it easier to use is not the answer.

Russ

[snaury, 2013-01-30 17:13:41]

On 2013/01/30 17:04:37, rsc wrote:
> I believe we've found a better solution in the Windows case.

Glad to hear that, I'll abandon these CLs then.

[snaury, 2013-01-30 17:15:21]

*** Abandoned ***