Re: [PATCH] treewide: Flush pcap and log files, if used, before exiting

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 2792 bytes --]

On Thu, Aug 14, 2025 at 07:24:19AM +0200, Stefano Brivio wrote:
> On Thu, 14 Aug 2025 07:12:55 +0200
> Stefano Brivio <sbrivio@redhat.com> wrote:
> 
> > On Thu, 14 Aug 2025 14:10:20 +1000
> > David Gibson <david@gibson.dropbear.id.au> wrote:
> > 
> > > On Wed, Aug 13, 2025 at 06:45:05PM +0200, Stefano Brivio wrote:  
> > > > I didn't imagine that occasionally truncated pcap and log files, as a
> > > > result of commit a9d63f91a59a ("passt-repair: use _exit() over
> > > > return"), would be such a big deal, until I tried to debug TCP issues
> > > > with this beauty:    
> > > 
> > > I think the problems are more introduced by the previous patch
> > > d0006fa784a7 ("treewide: use _exit() over exit()").  
> > 
> > Oops, right.
> > 
> > > > 
> > > >   while true; do ./pasta --trace -l /tmp/pasta.log -p /tmp/pasta.pcap --config-net -t 5555 -- socat TCP-LISTEN:5555 OPEN:/tmp/large.rcv,trunc & (sleep 0.3; socat -T2 OPEN:large.bin TCP:88.198.0.164:5555; ); wait; diff large.bin /tmp/large.rcv || break; done
> > > > 
> > > > ...flush files and pcap if we're using them. Ignore fsync() errors for
> > > > the log file as we obviously can't reliably log them.
> > > > 
> > > > Signed-off-by: Stefano Brivio <sbrivio@redhat.com>    
> > > 
> > > Hmmmmm.
> > > 
> > > I mean, yes, AFAICT this patch will address the immediate problem.
> > > But between this and 081df67d1fb2 ("conf: flush stdout before early
> > > exit") it seems more and more to me that _exit() just isn't what we
> > > want.  Basically the assertion in d0006fa784a7 that "no exit handlers
> > > are needed" doesn't really seem to be true.  
> > 
> > It was a wrong assumption but just for a couple of cases, and mind that
> > exit() doesn't give you any guarantee anyway. While glibc might
> > guarantee those flushes, we're not just building against glibc.
> > 
> > So, strictly speaking, at least for correctness, we should actually
> > keep those fflush() calls plus the ones I'm adding here, even with
> > exit().
> 
> Ah, wait, sorry, I just double checked and starting from Issue 7 of
> POSIX (referring to _exit() and _Exit()):
> 
>   Austin Group Interpretation 1003.1-2001 #031 is applied, separating
>   these functions from the exit() function.
> 
> and:
> 
>   The exit() function shall then flush all open streams with unwritten
>   buffered data and close all open streams.

Yeah, I was thinking that sounded wrong.  I wasn't sure where it would
be specified, but to not flush stdio streams on normal program exit
(i.e. exit(3)) would be Really Weird.

-- 
David Gibson (he or they)	| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you, not the other way
				| around.
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]