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

Run Recorder in a process instead of thread. #24

Merged
merged 1 commit into from Nov 16, 2019
Merged

Conversation

@cmangla
Copy link
Collaborator

cmangla commented Nov 16, 2019

Also serialise stop events. Both changes improve reliability.

Description

In Recorder.join(), Thread.join(...) hangs. It is safe to run Recorder in a different process instead of a thread, and that solves this issue.

Types of Changes

  • Core
  • Bugfix
  • New feature
  • Enhancement/optimization
  • Documentation

Issues Fixed or Closed by This PR

  • Recorder thread fails to join #23

Checklist

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • I have tested the changes and verified that they work and don't break anything (as well as I can manage).
Also serialise stop events. Both changes improve reliability.
@mre

This comment has been minimized.

Copy link
Owner

mre commented Nov 16, 2019

Tested and works. Thanks @cmangla. 😊
Along the way I noticed that the dependencies of the project are quite out-of-date. Will create a PR for that. Maybe you have time to be a reviewer?

@mre mre merged commit 97552cb into mre:master Nov 16, 2019
@cmangla

This comment has been minimized.

Copy link
Collaborator Author

cmangla commented Nov 16, 2019

Tested and works. Thanks @cmangla. 😊
Along the way I noticed that the dependencies of the project are quite out-of-date. Will create a PR for that. Maybe you have time to be a reviewer?

You're welcome @mre! Sure, I can review that.

@cmangla cmangla deleted the cmangla:recorder-process branch Nov 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.