[M3devel] cvsup fix refinements

Fri Mar 19 23:49:15 CET 2010

I was going to ask:

  Do you mean pthreads or Modula-3 threads?

  How you think it is or how you think it should be?

Modula-3 user threads are inherited.

That is what this function would say.

At least they are currently.

However, as you assert, I've also wondered many times to myself the past few days:

  Should Modula-3 userthreads and pthreads act the same way? Always? Optionally?

What stopped me was I couldn't convince myself it is trivial to implement.

I was thinking, like, you'd associate a pid with each thread. If getpid doesn't match thread.pid, the thread is from before a fork. I then worried that you'd build up all the data about threads "forever", across multiple forks. However that is probably false.

As well, if you provide RTProcess.Fork() as the replacement for fork(), it affords more power or at least ease of implementation.

I guess where you end is like:

RTProcess.Fork()

  does nothing on Windows 

     in fact is <* EXTERNAL *> and fails to link most likely  

     You could have the convention of returning ENOSYS but I say no.

     Or it might be a NIL pointer, following the convention you know about, have used several times. 

  just calls fork() on pthreads 

  on user threads, described later 

RTProcess.AtFork() (maybe RegisterAtFork or RegisterForkCallbacks?)

 does nothing on Windows (but exists and links, usable by portable code)  

    might be NIL 

 on pthreads just calls pthread_atfork 

 on user threads, described later   

And then RTProcess.Fork(), RTProcess.AtFork on user threads work together to provide basically the same meaning as fork() + pthread_fork() has on pthreads.

Easy to imagine how to code it actually. AtFork maintains an array of function pointers. Fork() calls them and fork in the proscribed order. The array is inherited across processes (like pthread_atfork()).

Only the calling thread survives, like fork().

I didn't realize it at first, but killing of all other threads is pretty easy.

You just reinitialize like I did for pthreads.

Then the next time the timer expires, it doesn't find them to be scheduled.

I also strongly suspect that in libm3 where fork+exec is called, some of what it does -- in particular turning off the timer -- would be moved to RTProcess.Fork().

Or perhaps that is in ThreadPosix.m3's AtForkPrepare.

Either way, the general idea is that fork is easier to use, be it fork + exec, or even fork + do work. You replace it with RTProcess.Fork().

As well however, fork + exec on pthreads continues to work.

You don't have to call RTProcess.Fork().

However RTProcess.Fork() is needed to provide matching semantics in user threads if you don't exec after fork. Geez this is a mouthful. I think it is somewhat important to dwell on -- what semantics change, which external libraries just work and don't have to be wrapped.

  if you are doing fork + exec, you can keep doing the same, I think

  if you don't care about user threads, and want them all to be inherited, you can keep doing the same, I think

No working code should have an altered meaning.

cvsupd on pthreads doesn't count as working code, of course.

In fact no fork + do work code on pthreads probably worked.

My assertion that "we should use pthread_atfork and be good pthread citizens" remains true and respected by these changes.

I'll code this up over the next few days/week.

I'm not sure anything ends up in Unix, could all end up in RTProcessC.c or such.

If RTProcess.i3 isn't public, it will be.

Or we can introduce a new interface.

This all sounds about right?

Notice that, like my second set of diffs, existing pthreads fork + exec paths will be "significantly" affected -- taking all the locks in the prepare call. It should work.

 - Jay

Subject: Re: [M3devel] cvsup fix refinements
From: hosking at cs.purdue.edu
Date: Fri, 19 Mar 2010 16:17:34 -0400
CC: m3devel at elegosoft.com
To: jay.krell at cornell.edu

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