Skip to content

Conversation

@bbakerman
Copy link
Member

See #1234

This is similar but more deliberate. That is we work out IF we have to dispatch inside the lock and then do the dispatch outside it.

I think this is a smidge cleaner than the other PR. (but the same effect)

@bbakerman bbakerman requested a review from andimarek October 2, 2018 07:15
@bbakerman
Copy link
Member Author

I was able to replicate 100% of the time without the change as per @tinnou original test

public void dispatchIfNotDispatchedBefore(int level, Runnable dispatch) {
public boolean dispatchIfNotDispatchedBefore(int level) {
if (dispatchedLevels.contains(level)) {
Assert.assertShouldNeverHappen("level " + level + " already dispatched");
Copy link
Member

Choose a reason for hiding this comment

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

you could actually return the Assert

@andimarek andimarek added this to the 11.0 milestone Oct 3, 2018
@bbakerman bbakerman merged commit 80342b2 into graphql-java:master Oct 3, 2018
@bbakerman bbakerman added the needs to be backported a bugfix that still needs to be backported label Oct 3, 2018
bbakerman added a commit to bbakerman/graphql-java that referenced this pull request Oct 3, 2018
* Added @OverRide as part of errorprone code health check

* Revert "Added @OverRide as part of errorprone code health check"

This reverts commit 38dfab1

* Brads attempt at graphql-java#1234

* Missed the test
@bbakerman bbakerman mentioned this pull request Oct 3, 2018
bbakerman added a commit that referenced this pull request Oct 3, 2018
* Added @OverRide as part of errorprone code health check

* Revert "Added @OverRide as part of errorprone code health check"

This reverts commit 38dfab1

* Brads attempt at #1234

* Missed the test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs to be backported a bugfix that still needs to be backported

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants