Skip to content

Conversation

@MadCoder
Copy link
Contributor

Also reorder all vtable entries to be in the same order (push, invoke,
wakeup).

Reenable dispatch_context_for_key.

SR: SR-4039

Also reorder all vtable entries to be in the same order (push, invoke,
wakeup).

Reenable dispatch_context_for_key.

SR: SR-4039
@MadCoder MadCoder requested review from das and dgrove-oss February 23, 2017 21:06
@MadCoder
Copy link
Contributor Author

@swift-ci please test

.do_dispose = _dispatch_queue_dispose,
.do_invoke = _dispatch_queue_invoke,
.do_push = _dispatch_queue_push,
.do_invoke = _dispatch_queue_invoke,
Copy link
Contributor

Choose a reason for hiding this comment

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

FWIW push -> wakeup -> invoke would have seemed the natural order to me ;-)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just kept the order that was dominating. For real the better order is:

wakeup -> push -> invoke. (cause it's in that order it happens)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

actually no I'm tired, it's push on self, wakeup onto target, then invoke you're correct :P

Copy link
Contributor

@dgrove-oss dgrove-oss left a comment

Choose a reason for hiding this comment

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

Looks good to me; I applied the fix of adding the missing .do_push entry to the queue_specific_queue vtable and have been run dispatch_context_for_key in loop without crashing for several thousand iterations. Was crashing in < 100 iterations before.

@das das merged commit 272e818 into master Feb 24, 2017
@das das deleted the eng/PR-SR-4039 branch February 24, 2017 02:24
das added a commit that referenced this pull request May 25, 2017
queue_specific_queue is missing its do_push vtable entry

Signed-off-by: Daniel A. Steffen <dsteffen@apple.com>
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.

4 participants