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

Use Vector for EventHandler (1.1.x) #4030

Merged
merged 2 commits into from Mar 23, 2018

Conversation

Projects
None yet
3 participants
@eatkins
Copy link
Contributor

commented Mar 21, 2018

ArrayList::add is not thread safe. I ran into cases where async tests
using utests would fail even when all of the individual tests passed.
This was because multiple threads called back into the handle method of
the handler instance variable, which just delegated to eventList::add.
When this happened, one of the events would get added to the list as a
null reference, which would manifest as an NPE upstream on the master
process. After this change, my tests stopped failing.

@eed3si9n eed3si9n added the ready label Mar 21, 2018

@eatkins eatkins force-pushed the eatkins:vector-1.1 branch from 5c490fc to d45ea2b Mar 21, 2018

Ethan Atkins
Add test for async utest TestSuites
Sometimes when utest runs async tests (i.e. tests that return a future)
the test suite will fail even when none of the individual tests do. This
is due to a data race in sbt. Most, but not all, of the time, this test
will induce that race.

@eatkins eatkins force-pushed the eatkins:vector-1.1 branch from d45ea2b to e2e3ca0 Mar 21, 2018

Ethan Atkins
Use ConcurrentLinkedDeque for EventHandler
ArrayList::add is not thread safe. I ran into cases where async tests
using utests would fail even when all of the individual tests passed.
This was because multiple threads called back into the handle method of
the handler instance variable, which just delegated to eventList::add.
When this happened, one of the events would get added to the list as a
null reference, which would manifest as an NPE upstream on the master
process. After this change, my tests stopped failing.

@eatkins eatkins force-pushed the eatkins:vector-1.1 branch from e2e3ca0 to 9b24e9f Mar 21, 2018

@dwijnand
Copy link
Member

left a comment

thank you @eatkins

@dwijnand dwijnand merged commit a43c18e into sbt:1.1.x Mar 23, 2018

3 checks passed

Codacy/PR Quality Review Good work! A positive pull request.
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@dwijnand dwijnand removed the ready label Mar 23, 2018

@dwijnand dwijnand added this to the 1.1.2 milestone Mar 23, 2018

@eed3si9n eed3si9n changed the title Vector 1.1 Use Vector for EventHandler (1.1.x) Mar 24, 2018

@eatkins eatkins deleted the eatkins:vector-1.1 branch Oct 9, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.