Skip to content

Make v2/Plugin accesses safe.#29191

Merged
anusha-ragunathan merged 1 commit intomoby:masterfrom
anusha-ragunathan:plugin_races
Dec 8, 2016
Merged

Make v2/Plugin accesses safe.#29191
anusha-ragunathan merged 1 commit intomoby:masterfrom
anusha-ragunathan:plugin_races

Conversation

@anusha-ragunathan
Copy link
Copy Markdown
Contributor

v2/Plugin struct had fields that were

  • purely used by the manager.
  • unsafely exposed without proper locking.
    This change fixes this, by moving relevant fields to the manager as well
    as making remaining fields as private and providing proper accessors for
    them.

Signed-off-by: Anusha Ragunathan anusha@docker.com

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

RLock?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Shouldn't accesses to the cMap be protected by the mutex?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, updated.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Could v2.Plugin reference controllers directly (probably through an interface) instead of using this map keyed on pointer values?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I'd suggest unembedding this field, to unexport it. It's probably not good for other packages to be able to lock and unlock the mutex directly.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated.

v2/Plugin struct had fields that were
- purely used by the manager.
- unsafely exposed without proper locking.
This change fixes this, by moving relevant fields to the manager as well
as making remaining fields as private and providing proper accessors for
them.

Signed-off-by: Anusha Ragunathan <anusha@docker.com>
@vdemeester
Copy link
Copy Markdown
Member

/cc @vieux @tiborvass

@vieux
Copy link
Copy Markdown
Contributor

vieux commented Dec 8, 2016

LGTM

1 similar comment
@tiborvass
Copy link
Copy Markdown
Contributor

LGTM

@anusha-ragunathan anusha-ragunathan merged commit 9d898b8 into moby:master Dec 8, 2016
@anusha-ragunathan anusha-ragunathan deleted the plugin_races branch December 16, 2016 22:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants