Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add some concepts related to exit(3) #17

Merged
merged 6 commits into from
Jan 19, 2023
Merged
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 40 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,46 @@ TID is a 32-bit integer to identify threads created with `wasi_thread_spawn`.
For example, it can be used to indicate the main thread, which doesn't
have a TID in the current version of this proposal.

### Process

* A process is a group of threads.

* The main thread starts with a process which only contains
the main thread.

* Threads created by a thread in a process using `wasi_thread_spawn`
is added to the process.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
is added to the process.
are added to the process.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done


* When a thread is terminated, it's removed from the process.

### Voluntary thread termination

A thread can terminate itself voluntarily by returning from
`wasi_thread_start`.

### Changes to WASI `proc_exit`

With this proposal, the `proc_exit` function takes extra responsibility
to terminate all threads in the process, not only the calling one.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about simply "When proc_exit is called the entire process is terminated, including all running threads". I'm not we need the "takes extra responsibility" wording.

It seems fairly clear from its name that proc_exit would do this, so I don't really see it as "extra responsibility", but maybe thats just my reading of it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

without wasi-threads, a runtime would simply terminate the calling thread.

when adding wasi-threads support to a runtime, you need to make it terminate other thread too. it would need a significant implementation effort. i feel it's appropriate to call it extra.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure. But don't think that is clear enough to simply say: "When proc_exit is called the entire process is terminated, including all running threads".

(BTW, the way I see it proc_exit brings down the entire process (that is that the proc part means), even without wasi-threads. It just happens that there was only ever one thread previously.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe. i wanted to somehow emphasize that this is something a runtime needs to implement.


Any of threads in the process can call `proc_exit`.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Any of threads in the process can call `proc_exit`.
Any of the threads in the process can call `proc_exit`.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done


### Traps

When a thread caused a trap, it terminates all threads in the process
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/caused/causes/ ?

How about "When a trap occurs in any thread, the entire process is terminated."?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/caused/causes/ ?

probably. i'm not good at english as you may noticed.

How about "When a trap occurs in any thread, the entire process is terminated."?

done.

similarly to `proc_exit`.

### Process exit status

If one or more threads call WASI `proc_exit` or raise a trap,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it include calling it proc_exit from the main thread?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes

one of them is chosen by the runtime to represent the exit status
of the process.
It's non deterministic which one is chosen.

If all the threads in the process have been terminated without calling
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about, "If that last running thread in a process terminates without calling proc_exit or trapping it is treated as if ... "

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's not only about the last running thread.

consider a process with two threads.

  1. a thread terminated itself by calling proc_exit(50)
  2. another thread, which is the last running thread, terminates itself by returning from wasi_thread_spawn before the runtime terminates it for proc_exit.
  3. the exit code of the process should be 50.

i think your wording can be interpreted as the exit code of the process can be 0.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My my understanding, in your example above proc_exit(50) causes all threads to be terminated, so its not possible (2) to occor.

Another way of putting it "proc_exitdoes not complete until all threads are terminated, so not thread can exist afterproc_exit` is complete.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My my understanding, in your example above proc_exit(50) causes all threads to be terminated, so its not possible (2) to occor.

if things work in a lockstep, maybe.
but we are talking about threads...

Another way of putting it "proc_exitdoes not complete until all threads are terminated, so not thread can exist afterproc_exit` is complete.

in that case, i guess you need to say the thread which called proc_exit is terminated after all other threads are terminated. i feel it's more complicated than the current sentence.

Copy link
Member

@sbc100 sbc100 Jan 11, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would odd if we allowed any threads to outlive the thread that calls proc_exit.. the whole point of that system call is to bring down all the threads so logically nothing else can happen on any thread once that syscall completes.

I don't want to hold up this PR, even though I don't love the way this sentence is phrased. I guess we can land this as it stands and try to improve it later.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, let me know if #27 addresses some of the things you bring up here...

`proc_exit` or raising a trap, it's treated as if the last thread called
`proc_exit` with exit code 0.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we not mention proc_exit here and just say that exit code is 0? It might get a bit confusing if e.g. in the future proc_exit is extended with additional functionality - in that case, this sentence can be interpreted as that additional functionality should also be executed along with returning 0 as exit code (whereas I think it shouldn't).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i can't imagine any functionalities which proc_exit might gain but on the other hand should not be a part of this. do you have examples?

even if we avoid mentioning proc_exit here, when you are adding an extra functionality to proc_exit, you still need to consider if the change should apply to this sentence or not.

Copy link
Collaborator

@loganek loganek Jan 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you have examples?

No, it was only hypothetical question.

you still need to consider if the change should apply to this sentence or not.

Yes


#### Design choice: pthreads

One of the goals of this API is to be able to support `pthreads` for C compiled
Expand Down