Skip to content

TestName: Make 'name' field volatile - #1157

Closed
rschmitt wants to merge 1 commit into
junit-team:masterfrom
rschmitt:test-name
Closed

TestName: Make 'name' field volatile#1157
rschmitt wants to merge 1 commit into
junit-team:masterfrom
rschmitt:test-name

Conversation

@rschmitt

@rschmitt rschmitt commented Jun 4, 2015

Copy link
Copy Markdown
Contributor

This ensures that the name is published across threads correctly--for
instance, if a parallelized runner is used.

This ensures that the name is published across threads correctly--for
instance, if a parallelized runner is used.
@kcooney kcooney closed this in eedd1f6 Jun 5, 2015
@kcooney

kcooney commented Jun 5, 2015

Copy link
Copy Markdown
Member

Merged. Thanks!

I think in practice, we were probably fine, since it would be hard for a thread that did not update that field to get a reference to the rule. But making it volatile is safer and there's almost no overhead on most processors.

@rschmitt

rschmitt commented Jun 5, 2015

Copy link
Copy Markdown
Contributor Author

Suppose you have a Timeout downstream of TestName. The name field will be set, and then Timeout will spawn a separate thread in which the actual test will run. Currently this should be safe because it creates a new Thread object for every test invocation, but if this implementation detail ever changed (e.g. Timeout reuses threads in a thread pool) undefined behavior will result.

@kcooney

kcooney commented Jun 5, 2015

Copy link
Copy Markdown
Member

Ah, I see. I could easily imagine custom Timeout implementations that use a thread pool

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.

2 participants