facebookarchive / swift

An annotation-based Java library for creating Thrift serializable types and services.
Apache License 2.0
900 stars 297 forks source link

Fix recursive type stack double-push, double-pop bug #306

Closed andrewcox closed 8 years ago

andrewcox commented 8 years ago

Code on 260 would pop the list, then 262 would pop it again.

Code for getThriftTypeReference already adds the type to the worklist, so it was added twice for all cases I've seen. But there are complex cases where I was calling getThriftTypeReference directly w/o pushing separately, and so it's possible to skip two types in the worklist and only process one.

The way I handled types in collections that were potentially recursive, this unfortunately affects even non-recursive types :(

This is the fix.

alandau commented 8 years ago

Lgtm

On Jun 3, 2016 5:23 PM, andrewcox notifications@github.com wrote:

Code on 260 would pop the list, then 262 would pop it again.

Code for getThriftTypeReference already adds the type to the worklist, so it was added twice for all cases I've seen. But there are complex cases where I was calling getThriftTypeReference directly w/o pushing separately, and so it's possible to skip two types in the worklist and only process one.

The way I handled types in collections that were potentially recursive, this unfortunately affects even non-recursive types :(

This is the fix.


You can view, comment on, or merge this pull request online at:

https://github.com/facebook/swift/pull/306

Commit Summary

File Changes

Patch Links:

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHubhttps://github.com/facebook/swift/pull/306, or mute the threadhttps://github.com/notifications/unsubscribe/AEHUuXePhcc4VLOwiX9HB0YJ1YXaqmclks5qIMVvgaJpZM4IuATk.

andrewcox commented 8 years ago

Merged and pushed