Skip to content

Add lifetime management to threads #1785

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

Merged
merged 2 commits into from
Jun 1, 2016
Merged

Conversation

geky
Copy link
Contributor

@geky geky commented May 25, 2016

Problem:

  • Threads begin running when constructed, allocation and execution are unnecessarily coupled
  • Errors on starting threads can not be easily propagated
  • Exacerbates issues with rtos initialization
  • Threads lack join method, encouraging termination which can lead to very strange bugs

Proposed solution:

  • Add Thread::start for explicitly starting threads and handling errors
  • Defer to Thread::start from constructor if function is provided, keeping backwards compatibility
  • Add Thread::join for gracefully merging threads

Thread::start in other languages: c++, c, java, python
Thread::join in other languages: c++, c, java, python

cc @niklas-arm, @sg-, @c1728p9, @0xc0170
mv #79

- Allows threads to started separately from when they are declared,
  avoiding the need to dynamically allocate threads at runtime.
- Allows multiple threads to join without forceful termination. Allowing
  threads to synchronize on cleanup.
@@ -32,34 +32,82 @@ extern "C" P_TCB rt_tid2ptcb(osThreadId thread_id);

namespace rtos {

Thread::Thread(osPriority priority,
uint32_t stack_size, unsigned char *stack_pointer):
_tid(NULL), _dynamic_stack(stack_pointer == NULL) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This needs to be _tid((osThreadId)NULL), ... instead. After that travis will pass.

@geky
Copy link
Contributor Author

geky commented May 26, 2016

The null issue should be fixed now.

I also added you guys as collaborators on my fork, so feel free to squash the commits if you like.

@0xc0170 0xc0170 merged commit 2db84da into ARMmbed:master Jun 1, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants