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

Iqlusioninc abscissa/v0.5 #169

Merged
merged 4 commits into from
Jan 15, 2020
Merged

Iqlusioninc abscissa/v0.5 #169

merged 4 commits into from
Jan 15, 2020

Conversation

hdevalence
Copy link
Contributor

Supersedes #155 because I still don't know how to make "edits from maintainers" on GitHub....

This adds a single commit that fixes a deadlock in the TokioComponent.

The components are accessed by a lock on application state. When some command
calls block_on to enter an async context, it obtained a write lock on the
entire application state. This meant that if the application state were
accessed later in an async context, a deadlock would occur. Instead the
TokioComponent holds an Option now, so that before calling block_on,
the caller can .take() the runtime and release the lock. Since we only ever
enter an async context once, it's not a problem that the component is then
missing its runtime, as once we are inside of a task we can access the runtime.

@hdevalence
Copy link
Contributor Author

CI failed because I pulled out the branch from under it to rebase onto main, rerunning....

@hdevalence
Copy link
Contributor Author

Hmm, actually it looks like there might be a problem with some GCloud thing rather than just the force-pushing 🤔

@hdevalence
Copy link
Contributor Author

This is failing because the tracing component is now outputting log information to a test that enforces that the version command only outputs the version string.

tony-iqlusion and others added 4 commits January 15, 2020 11:56
The components are accessed by a lock on application state.  When some command
calls block_on to enter an async context, it obtained a write lock on the
entire application state.  This meant that if the application state were
accessed later in an async context, a deadlock would occur.  Instead the
TokioComponent holds an Option<Runtime> now, so that before calling block_on,
the caller can .take() the runtime and release the lock.  Since we only ever
enter an async context once, it's not a problem that the component is then
missing its runtime, as once we are inside of a task we can access the runtime.
@hdevalence
Copy link
Contributor Author

I disabled the test because I don't want this to block on combining all of the tracing component configs with the zebrad configs.

@dconnolly dconnolly self-requested a review January 15, 2020 20:03
Copy link
Contributor

@dconnolly dconnolly left a comment

Choose a reason for hiding this comment

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

LGTM!


/*
* Disabled pending tracing config rework, so that merging abscissa fixes doesn't block on this
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

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.

None yet

3 participants