[M3devel] cvsup fix refinements

Fri Mar 19 19:50:01 CET 2010

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/d51649a3/attachment-0002.html>