Hi Richard,

On Wed, Feb 20, 2019 at 11:58:42PM +0000, Richard Russo wrote:
> While continuing to test this, I ended up with a crash in 
> listener.c:listener_accept on a closed/closing listen socket where 
> fdtab[fd].owner is NULL by the time the thread gets there. This is possible 
> because the fd.c: fdlist_process_cached_events unlocks the spinlock before 
> calling fdtab[fd].iocb(fd); allowing for a concurrent close.
> 
> I'm not sure if the right thing to do is to hold the FD's spinlock while 
> calling the callback, consider a read/write style lock for listen FDs, or 
> simply adding a check that l is not NULL in listener_accept similar to the 
> one in connection.c:conn_fd_handle. I'm testing the NULL check right now, 
> since it's simplest. I haven't had a crash since, but that doesn't mean it 
> won't crash.  This change would need to be applied to 1.8 as well, although I 
> haven't managed to observe a crash on 1.8.

I think you're right on all accounts.
I've pushed your patch, and after talking with Willy, came to the conclusion
checking if the listener is NULL before using it should be enough, so I
pushed a patch that did just that as well.
Can you confirm that your problems are gone in master ?

Thanks a lot !

Olivier


Reply via email to