[M3devel] cvsup fix refinements
Tony Hosking
hosking at cs.purdue.edu
Fri Mar 19 21:17:34 CET 2010
I meant for both user and system threads.
On 19 Mar 2010, at 15:56, Jay K wrote:
> Agreed. Details later.
>
> From: hosking at cs.purdue.edu
> Date: Fri, 19 Mar 2010 14:50:01 -0400
> To: jay.krell at cornell.edu
> CC: m3devel at elegosoft.com
> Subject: Re: [M3devel] cvsup fix refinements
>
> The way fork is defined on modern platforms I don't think the child should ever inherit all the threads.
>
> On 19 Mar 2010, at 14:47, Jay K wrote:
>
> How about DoesForkExist, DoesForkInheritAllThreads?
>
>
> From: jay.krell at cornell.edu
> To: hosking at cs.purdue.edu
> Date: Fri, 19 Mar 2010 18:15:34 +0000
> CC: m3devel at elegosoft.com
> Subject: Re: [M3devel] cvsup fix refinements
>
> RTProcess is fine.
> Unix is a little wierd because I don't think it should do anything if using userthreads.
> Unix implies a fairly thin wrapper?
>
> - Jay
>
>
> Subject: Re: [M3devel] cvsup fix refinements
> From: hosking at cs.purdue.edu
> Date: Fri, 19 Mar 2010 14:13:32 -0400
> CC: m3devel at elegosoft.com
> To: jay.krell at cornell.edu
>
> Why not put it into RTProcess?
> Or into Unix?
> I want to avoid confusion between Thread.Fork (fork a thread) and process fork.
>
> On 19 Mar 2010, at 14:08, Jay K wrote:
>
> > could go in any of ThreadF, RTOS, RTProcess?
>
>
> Or RTThread. Or almost anywhere.
>
>
> Some typos in the diffs:
> in a comment, ThreadF__ should be RTOS__
> function PThreadForkHandler named inconsistently,
> should be PThreadAtForkHandler
>
>
> Again, "PThread" appears in these names to indicate
> their meaning is not portable. Their existance is, but not their meaning.
> We have no way of saying "only call this function for pthreads",
> instead as I understand, we provide a portable interface with
> drastically varying semantics, such as "do something" vs. "do nothing".
>
>
> Given that Init in ThreadPThread.m3 is private, we could probably
> take the stackbase parameter from the caller instead.
>
>
> The call to AtFork should probably follow InitWithStackbase,
> in case it fails under low memory, the Die/assert more likely to "work".
>
>
> I'm not completely happy making RTOS public.
> So maybe ThreadF?
> I had gone to RTOS because SetNextForkWillExec required LockHeap/UnlockHeap.
> But for now they don't exist.
>
>
> Maybe a new interface? ThreadPThread.i3, but is still
> present on all platforms, so mostly portable can call it.
>
>
> ThreadPThread.AtFork?
> PThread.AtFork? That seems right.
>
>
> - Jay
>
>
> From: jay.krell at cornell.edu
> To: m3devel at elegosoft.com
> Date: Fri, 19 Mar 2010 17:21:13 +0000
> Subject: Re: [M3devel] cvsup fix refinements
>
> diffs attached
>
>
> For now I've removed the Get/SetNextForkWillExec.
> I also think truly get/set is right, not inc/dec.
> It was confusing enough to interpret and initialize
> the value. And it is also not clear what the default
> should be. The usual behavor is exec does follow fork.
> The safer default if the handlers work ok is to assume
> exec will not follow. It is a perf hint or a don't change
> how things were hint? Hint to speed up fork+exec or hint
> to avoid running the new code that might not work?
>
>
> (For now I've removed the Get/SetNextForkWillExec.)
> They'd have to be implemented three times, and
> I had trouble defining them.
> Obviously Win32/userthreads just return a
> hardcoded true, but it is hard to explain
> from the caller of Get's point of view.
>
> Maybe:
> SetNextForkWillExec
> GetNextForkWillExec
>
> and then:
> PThreadAtForkNeeded() = GetNextForkWillExec = FALSE ?
>
> That is,
> someone is who is calling fork and possibly exec,
> can immediately tell what these functions mean.
>
>
> But the implementer of an atfork handler has to
> do quite a semantic translation I think.
> I wrote it backwards the first time. That either
> implies it is confusing, or I wasn't thinking.
>
>
> If it really is a problem to run this stuff when exec
> will follow, we should know quickly, as building cm3
> is an aggressive user of this code.
>
>
> This version has worked repeatedly on Darwin.
> I didn't test user threads but it is structured such
> as to make that obviously work.
> The cvsup changes are in pthreaad_atfork handlers,
> so no change for userthreads.
>
>
> I didn't test on others but confidence is quite high now.
>
> description of changes at least where they are hard to read:
>
> m3core:
> Init split into Init and InitWithStackBase,
> We have to provide the old stack base in the child fork handler
> instead of estimating from the address of a local. I don't
> know what stack is used, maybe the caller of fork?
> i.e. we might have eaten a fair amount of stack and
> there could be arbitrary references above the current stack.
>
>
> WeakRefFromRef split into WeakRefFromRef and StartWeakCleaner
>
> There is a resulting double lock in WeakRefFromRef, every time.
> Could instead copy/paste more code around, i.e.:
> EVAL Thread.Fork(NEW(Thread.Closure, apply := WeakCleaner));
>
>
> - Jay
>
>
>
>
>
>
> From: jay.krell at cornell.edu
> To: m3devel at elegosoft.com
> Date: Fri, 19 Mar 2010 16:01:03 +0000
> Subject: [M3devel] cvsup fix refinements
>
> refinements:
>
>
> new public functions in m3core:
>
>
> TYPE PThreadForkHandler = PROCEDURE();
>
> <* EXTERNAL RTOS__PThreadAtFork *>
> PROCEDURE PThreadAtFork(prep, parent, child: PThreadForkHandler): INTEGER;
>
>
> (That's not a change yet.)
>
>
> PROCEDURE SetNextForkWillExec(value: BOOLEAN);
>
> PROCEDURE GetNextForkWillExec(): BOOLEAN;
>
> These are only an optimization and should
> perhaps be removed? They should be used
> under LockHeap/UnlockHeap? which wasn't previously
> public. This way, existing fork/exec paths
> not affected, though maybe though might as well
> ought to be?
>
>
> could go in any of ThreadF, RTOS, RTProcess?
>
>
> Should be called under RTOS.LockHeap?
> Therefore I put in RTOS.
> Also helps that RTOS is exported from *Thread.m3.
> RTOS not previously public.
>
>
> Should Get/Set be Inc/Dec? (therefore just Inc with +1,-1?)
> I implemented them that way: true incs, false decs.
>
>
> Or don't bother with them at all?
>
>
> Furthermore, in RTCollector, instead of claiming the threads
> aren't started, if they were started, restart them.
> This makes more sense for the weak cleaner to me, and
> seems reasonable for the other two.
>
>
> Diffs not yet available.
>
>
> - Jay
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://m3lists.elegosoft.com/pipermail/m3devel/attachments/20100319/2c696286/attachment-0002.html>
More information about the M3devel
mailing list