Re: race condition in libports

2017-12-17 Thread Brent W. Baccala 
On Sun, Dec 17, 2017 at 8:02 AM, Samuel Thibault 
wrote:

> Hello,
>
> Brent W. Baccala, on sam. 16 déc. 2017 21:37:05 -0500, wrote:
> > basically, we're storing ports in a inode-to-port hash, looking them
> > up when io_identity() gets called, and removing them from the hash
> > when the class's clean routine gets called.
>
> That's the bug: one needs a reference for this.  And it's a weak
> reference: identity is fine with getting rid of it.  Could you try the
> attached patch?


Makes sense.  I've applied it.  So far, so good.

Now I feel stupid!  Holding a port in a hash is such a classic example of a
weak reference, but I didn't even think about moving that code out of the
clean function.

Thank you.

agape
brent

Re: [PATCH] LwIP translator

2017-12-17 Thread Samuel Thibault 
Hello,

I'm however having a hard time matching your
g...@github.com:jlledom/liblwip-hurd.git repository with the upstream
https://git.savannah.nongnu.org/git/lwip.git repository. Since the plan
is to just pick up upstream as much as possible, we need to keep the
delta small.

Or are you actually based on another lwip?

Samuel

Re: [PATCH] LwIP translator

2017-12-17 Thread Samuel Thibault 
Hello,

At last, found the time to commit this :)

Huge hanks!
Samuel

Re: race condition in libports

2017-12-17 Thread Samuel Thibault 
Hello,

Brent W. Baccala, on sam. 16 déc. 2017 21:37:05 -0500, wrote:
> basically, we're storing ports in a inode-to-port hash, looking them
> up when io_identity() gets called, and removing them from the hash
> when the class's clean routine gets called.

That's the bug: one needs a reference for this.  And it's a weak
reference: identity is fine with getting rid of it.  Could you try the
attached patch?

Samuel
Index: hurd-debian/libfshelp/get-identity.c
===
--- hurd-debian.orig/libfshelp/get-identity.c
+++ hurd-debian/libfshelp/get-identity.c
@@ -42,7 +43,12 @@ id_clean (void *cookie)
 {
   struct idspec *i = cookie;
   pthread_mutex_lock ();
-  hurd_ihash_locp_remove (, i->id_hashloc);
+  if (refcounts_hard_references(>pi.refcounts) == 0)
+{
+  /* Nobody got a send right in between, we can remove from the hash.  */
+  hurd_ihash_locp_remove (, i->id_hashloc);
+  ports_port_deref_weak (>pi);
+}
   pthread_mutex_unlock ();
 }
 
@@ -50,7 +56,7 @@ static void
 id_initialize ()
 {
   assert_backtrace (!idclass);
-  idclass = ports_create_class (id_clean, NULL);
+  idclass = ports_create_class (NULL, id_clean);
 }
 
 error_t
@@ -75,6 +81,9 @@ fshelp_get_identity (struct port_bucket
   if (err)
 goto lose_port;
 
+  /* Weak reference for the hash entry.  */
+  ports_port_ref_weak(>pi);
+
   *pt = ports_get_right (i);
   ports_port_deref (i);
 }