Hey Samuel,

Thanks for the merge.
yeah we don't have to refcounts_deref_weak on refcounts, it won't leak.

But i see this underneath:

if (pi->class->clean_routine) (*pi->class->clean_routine)(pi);

and it is class specific so..

My thinking is the future clean_routine could possibly want to do some
defensive check inside that function that its fully unreferenced
(result.hard == 0 && result.weak == 0).
Dropping it to 0 guarantees the state is completely clean before handing
the struct off to arbitrary class logic.
And perhaps it will look cleaner on some future memory analyzer if it is 0?

Either way we are good.
Thanks again.

Milos

On Wed, Sep 16, 2026 at 4:20 PM Samuel Thibault <[email protected]>
wrote:

> Hello,
>
> Milos Nikic, le mer. 16 sept. 2026 11:24:41 -0700, a ecrit:
> > Here is a patch attached that tries to do what you suggested.
>
> Applied, thanks!
>
> > @@ -33,18 +33,18 @@ _ports_complete_deallocate (struct port_info *pi)
> >
> >        pthread_rwlock_wrlock (&_ports_htable_lock);
> >        refcounts_references (&pi->refcounts, &result);
> > -      if (result.hard > 0 || result.weak > 0)
> > +      if (result.hard > 0 || result.weak > 1)
> >          {
> >            /* A reference was reacquired through a hash table lookup.
> > -             It's fine, we didn't touch anything yet. */
> > -          /* XXX: This really shouldn't happen.  */
> > -          assert_backtrace (! "reacquired reference w/o send rights");
> > +             It's fine, we didn't touch anything yet.  */
> >            pthread_rwlock_unlock (&_ports_htable_lock);
> >            return;
> >          }
> >
> >        hurd_ihash_locp_remove (&_ports_htable, pi->ports_htable_entry);
> >        hurd_ihash_locp_remove (&pi->bucket->htable, pi->hentry);
> > +      /* Drop the reference held by the hash tables.  */
> > +      refcounts_deref_weak (&pi->refcounts, &result);
>
> It doesn't hurt, but I don't think it is really needed to drop that weak
> ref? We free pi below anyway. Or is there some other check below?
>
> Samuel
>

Reply via email to