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

fix NullReferenceException in task hub worker shutdown #1546

Merged
merged 3 commits into from
Nov 16, 2020

Conversation

sebastianburckhardt
Copy link
Collaborator

I noticed that the task hub worker does not shut down correctly if some functions are disabled with the [Disable] attribute.

The reason is that the info variable can be null in the following checks that happen during shutdown:

 if (this.isTaskHubWorkerStarted &&
                    !this.knownOrchestrators.Values.Any(info => info.HasActiveListener) &&
                    !this.knownActivities.Values.Any(info => info.HasActiveListener) &&
                    !this.knownEntities.Values.Any(info => info.HasActiveListener))

As far as I can see this is easily fixed by ignoring null entries for the purpose of this check.

@cgillum
Copy link
Collaborator

cgillum commented Nov 3, 2020

Oh that's interesting. I thought we had already covered the case where functions where disabled. At least, that's what I had in this comment here:

// This flag is set when a function is disabled or the host is shutting down.
internal bool IsDeregistered { get; set; }

So either I tested this wrong originally, or something changed, but it's very clear from the code that nulls can exist here. Thanks for submitting this fix! Maybe you can add a comment above your change explaining that null is expected for disabled functions?

Copy link
Collaborator

@cgillum cgillum left a comment

Choose a reason for hiding this comment

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

I like your local function approach. Very clean and easy to understand. :) LGTM as long as the CI is green.

@cgillum cgillum added this to the Extension v.2.4.0 milestone Nov 12, 2020
@sebastianburckhardt sebastianburckhardt merged commit 92edfbf into dev Nov 16, 2020
@sebastianburckhardt sebastianburckhardt deleted the sebastian/fix-nullref-in-shutdown branch November 16, 2020 23:25
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

2 participants