Skip to content

WithTag() makes contexts thread-unsafe #8

@mikeatgravity

Description

@mikeatgravity

WithTag() attempts to reuse the ctxOptions and its member tags in the context. But this is thread hostile:

Consider the case where a context is passed to multiple goroutines (either directly or via wrapping in someother context)). If the parent context already had a ctxOptions installed, then each of the children will overwrite the tags there, and worse do it without any mutex. This can lead to panics, e.g. when the Tags object is being iterated over in one goroutine while being updated in another. Or it can lead to subtle mistagging of database requests.

Some possible solutions:

A) Copy the options and map in each call to WithTag(). Con: This is more for each tag added, could be mitigated by adding a WithTags(ctx context.Context, tags Tags) method which allows multiple tags to be added at a time. (Which is common in the code I am working with)
B) Chain the options structs/tags. Con: this is more work to return the tags. (But I believe that this is how context.Value() is implemented, so maybe it isn't much worse)
C) Add an explicit WithForkedTags(ctx context.Contex) which does the copy, and expect callers to call it before copying the context for another goroutine. Cons this requires code that is not local to the sqlcomments to correctly copy the context.

I weakly prefer (A) over (B). I do not like (C) but include it for completeness.

I would be willing to send a PR if you are interested.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions