race condition destroying condition variables

[Brent W. Baccala]

Hi -

Looks like there's a race condition when we destroy a condition variable.
My understanding of the expected behavior is that once all the threads have
been signaled (i.e, pthread_cond_broadcast is called), the condition
variable can be safely destroyed with pthread_cond_destroy.

The problem is in glibc's libpthread/sysdeps/generic/pt-cond-timedwait.c.
After __pthread_block() returns, we spinlock on cond->__lock.  The problem
is that our __pthread_block() is just a mach_msg receive, and our
__pthread_wakeup() (called by pthread_cond_broadcast) is just a mach_msg
send.

So we can do a pthread_cond_broadcast, which will send messages to all
waiting threads, but there's no guarantee that the threads have received
the message; the message could be queued.  Then we destroy the condition
variable, then the thread receives the message and tries on spinlock on a
free'd region of memory.

It looks like the whole reason for that spinlock is to figure out if
somebody else removed us from the wait queue, and to remove ourselves from
the wait queue if they did not (i.e, we timed out).

I'm puzzling about how to fix it, other than by reorganizing my libpager
code so that condition variables don't get destroyed very often.

Also, I'm a bit confused by the management of the source code.  Is the
authoritative copy at git://git.savannah.gnu.org:/hurd/libpthread.git?

agape
brent

Re: [PATCH] LwIP translator

[Joan Lledó]

2017-12-18 17:28 GMT+01:00 Samuel Thibault :
> BTW, your lwip code should probably be made BSD-licensed, just like the
> rest of lwip.

Mmm, I think I don't know what you mean, if I send patches to a
BSD-licensed project, they are BSD too, right?

Re: [PATCH] LwIP translator

[James Clarke]

On 18 Dec 2017, at 17:09, Samuel Thibault  wrote:
> James Clarke, on lun. 18 déc. 2017 17:06:28 +, wrote:
>> On 18 Dec 2017, at 16:28, Samuel Thibault  wrote:
>>> Joan Lledó, on lun. 18 déc. 2017 17:10:42 +0100, wrote:
 2017-12-18 14:46 GMT+01:00 Samuel Thibault :
> We need to know what is not yet in upstream, what will
> eventually get to upstream, and what may not get to
> upstream.
 
 There're also some patches that are in upstream, I think it would be
 simpler for me to first upgrade liblwip to 2.0.3 and then take that
 version as start point to create the list of pending patches.
>>> 
>>> Ok.  In the meanwhile I have prepared an upload for lwip to Debian, I
>>> have borrowed some code I could find in your tree and hacked a bit, see 
>>> 
>>> git clone https://anonscm.debian.org/collab-maint/lwip.git
>> 
>> You missed the initial /git/ there (and in Vcs-Git).
> 
> It looks like dh-make is bogus then, that's what set this URL.

*sigh* Fixed in 2016, but no upload yet[1] (also [2] more recently).
I will ping the bug...

James

[1] 
https://anonscm.debian.org/cgit/collab-maint/dh-make.git/commit/?id=8bc8e0fab2f8293f8946e51768802532a6470d7f
[2] 
https://anonscm.debian.org/cgit/collab-maint/dh-make.git/commit/?id=b5a2903efa716e898c141397a6e471ec6fd9febb

Re: [PATCH] LwIP translator

[Samuel Thibault]

James Clarke, on lun. 18 déc. 2017 17:06:28 +, wrote:
> On 18 Dec 2017, at 16:28, Samuel Thibault  wrote:
> > Joan Lledó, on lun. 18 déc. 2017 17:10:42 +0100, wrote:
> >> 2017-12-18 14:46 GMT+01:00 Samuel Thibault :
> >>> We need to know what is not yet in upstream, what will
> >>> eventually get to upstream, and what may not get to
> >>> upstream.
> >> 
> >> There're also some patches that are in upstream, I think it would be
> >> simpler for me to first upgrade liblwip to 2.0.3 and then take that
> >> version as start point to create the list of pending patches.
> > 
> > Ok.  In the meanwhile I have prepared an upload for lwip to Debian, I
> > have borrowed some code I could find in your tree and hacked a bit, see 
> > 
> > git clone https://anonscm.debian.org/collab-maint/lwip.git
> 
> You missed the initial /git/ there (and in Vcs-Git).

It looks like dh-make is bogus then, that's what set this URL.

> Also there's already src:lwipv6 in the archive which last saw an
> upload in 2012, so might be worth unifying them if possible

Sure.

Samuel

Re: [PATCH] LwIP translator

[James Clarke]

On 18 Dec 2017, at 16:28, Samuel Thibault  wrote:
> Joan Lledó, on lun. 18 déc. 2017 17:10:42 +0100, wrote:
>> 2017-12-18 14:46 GMT+01:00 Samuel Thibault :
>>> We need to know what is not yet in upstream, what will
>>> eventually get to upstream, and what may not get to
>>> upstream.
>> 
>> There're also some patches that are in upstream, I think it would be
>> simpler for me to first upgrade liblwip to 2.0.3 and then take that
>> version as start point to create the list of pending patches.
> 
> Ok.  In the meanwhile I have prepared an upload for lwip to Debian, I
> have borrowed some code I could find in your tree and hacked a bit, see 
> 
> git clone https://anonscm.debian.org/collab-maint/lwip.git

You missed the initial /git/ there (and in Vcs-Git). Also there's already
src:lwipv6 in the archive which last saw an upload in 2012, so might be worth
unifying them if possible (it seems to be a fork from a while ago[1], probably
before IPv6 was added upstream).

Regards,
James

[1] https://sourceforge.net/p/view-os/code/HEAD/tree/trunk/lwipv6/

Re: [PATCH] LwIP translator

[Samuel Thibault]

Joan Lledó, on lun. 18 déc. 2017 17:10:42 +0100, wrote:
> 2017-12-18 14:46 GMT+01:00 Samuel Thibault :
> > We need to know what is not yet in upstream, what will
> > eventually get to upstream, and what may not get to
> > upstream.
> 
> There're also some patches that are in upstream, I think it would be
> simpler for me to first upgrade liblwip to 2.0.3 and then take that
> version as start point to create the list of pending patches.

Ok.  In the meanwhile I have prepared an upload for lwip to Debian, I
have borrowed some code I could find in your tree and hacked a bit, see 

git clone https://anonscm.debian.org/collab-maint/lwip.git

BTW, your lwip code should probably be made BSD-licensed, just like the
rest of lwip.

Samuel

Re: [PATCH] LwIP translator

[Joan Lledó]

2017-12-18 14:46 GMT+01:00 Samuel Thibault :
> Ok, but I don't see that base in your repository, the first commit
> already has a lot of source code which is not in 2.0.1.

Oh, yes. That's because I first imported all lwip 2.0.1 code to my
lwip-hurd repository and made many changes there. Then I moved it to
liblwip-hurd and made more changes. It's a bit of a mess, I'm sorry.

> We need to know what is not yet in upstream, what will
> eventually get to upstream, and what may not get to
> upstream.

There're also some patches that are in upstream, I think it would be
simpler for me to first upgrade liblwip to 2.0.3 and then take that
version as start point to create the list of pending patches.

Re: [PATCH] LwIP translator

[Samuel Thibault]

Hello,

Joan Lledó, on lun. 18 déc. 2017 12:58:35 +0100, wrote:
> 2017-12-18 2:32 GMT+01:00 Samuel Thibault :
> > Or are you actually based on another lwip?
> 
> Yes, It's based in the version 2.0.1, as I started to work on this
> back in February.

Ok, but I don't see that base in your repository, the first commit
already has a lot of source code which is not in 2.0.1.  We need to know
what is not yet in upstream, what will eventually get to upstream, and
what may not get to upstream.

Samuel

Re: [PATCH] LwIP translator

[Joan Lledó]

Hello,

2017-12-18 2:32 GMT+01:00 Samuel Thibault :
> Or are you actually based on another lwip?

Yes, It's based in the version 2.0.1, as I started to work on this
back in February. I plan to upgrade liblwip to the last version in a
few months.