From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from duke.felloff.net ([216.126.196.34]) by ewsd; Mon Dec  9 07:00:16 EST 2019
Message-ID: <C6EF2FB7E3D3C81FC5D7EB8C2DC9AE56@felloff.net>
Date: Mon, 9 Dec 2019 13:00:06 +0100
From: cinap_lenrek@felloff.net
To: 9front@9front.org
Subject: Re: [9front] [patch] upas: plumb flag updates on messages.
In-Reply-To: 27DBD03EAF07864ED33411518EE16A58@eigenstate.org
MIME-Version: 1.0
Content-Type: text/plain; charset="US-ASCII"
Content-Transfer-Encoding: 7bit
List-ID: <9front.9front.org>
List-Help: <http://lists.9front.org>
X-Glyph: ➈
X-Bullshit: converged open-source reduce/map out-scaling XMPP over SVG rich-client template-aware control 

ok, small remarks.

 char*
 syncmbox(Mailbox *mb, int doplumb)
 {
@@ -90,16 +87,14 @@
 				cachehash(mb, m);
 				m->cstate |= Cnew;
 				n++;
-			} else if(!doplumb)
-				m->cstate &= ~Cnew;
-			if(m->cstate & Cnew){
-				if(ensurecache(mb, m) == 0){
-					if(doplumb)
-						mailplumb(mb, m);
+			}
+			if(doplumb){
+				if((m->cstate & Cnew) && ensurecache(mb, m) == 0)
 					msgdecref(mb, m);
-				}
-				m->cstate &= ~Cnew;
+				if(m->cstate & (Cnew|Cmod))
+					mailplumb(mb, m);
 			}
+			m->cstate &= ~(Cnew|Cmod);
 		}
 		if(m->cstate & Cidxstale)

i think you want to do the msgdecref() of the message *after* mailplumb(),
not before. otherwise its like free before use. i know with the current
code it will still work but it is semantially wrong. if we later change
how cache eviction works that will blow up.

diff -r 7f3f8606fc12 sys/src/cmd/upas/fs/fs.c
--- a/sys/src/cmd/upas/fs/fs.c	Sun Dec 08 11:58:52 2019 -0800
+++ b/sys/src/cmd/upas/fs/fs.c	Sun Dec 08 11:59:39 2019 -0800
@@ -337,14 +337,15 @@
 		mboxfile = mbox;
 	}
 
-	if(mboxfile != nil)
-		if(err = newmbox(mboxfile, "mbox", 0, nil))
-			sysfatal("opening %s: %s", mboxfile, err);
 
 	switch(rfork(RFFDG|RFPROC|RFNAMEG|RFNOTEG|RFREND)){
 	case -1:
 		error("fork");
 	case 0:
+		print("child\n");
+		if(mboxfile != nil)
+			if(err = newmbox(mboxfile, "mbox", 0, nil))
+				sysfatal("opening %s: %s", mboxfile, err);
 		henter(PATH(0, Qtop), dirtab[Qctl],
 			(Qid){PATH(0, Qctl), 0, QTFILE}, nil, nil);
 		close(p[1]);

theres a debug print left there and this changes some semantics.
i know this is because initial sync from imap takes forever. but
people sometimes want to specify mailbox path at the initial
command line and expect error status if the initial sync failed
(like if mounting a mbox file in a script to parse a message).

i think a good compromise would be to add a commandline flag to
specify if you want initial sync in the background or not
(can then be added to /rc/bin/startupasfs).

if we do background syncing, then even if the initial sync
fails, it should try to continue. imagine your imap server
is currently down or network is unavailable, you want it to
continue trying in that case. and sysfataling here makes no
sense as nobody will be listening.

--
cinap