dave-hillier / disruptor-unity3d

Basic implementation of Disruptor for Unity3d
Apache License 2.0
200 stars 35 forks source link

Bounds safety in CopyTo #6

Closed wallstop closed 7 years ago

wallstop commented 7 years ago

CopyTo is used internally by ToArray.

Consider the following:

T0: ToArray is called. An array of size n, where n == count, is allocated, and passed to CopyTo to fill T1: An element is enqueued, count is increased by one T2: CopyTo walks off the end of the array, throwing an IndexOutOfRangeException

An alternative solution would be to leave CopyTo alone and simply delete .ToArray(), relying on the LINQ extension to provide proper support.

dave-hillier commented 7 years ago

This is taken directly from Mono and not intended to be a distribution of that file. It was included for comparison purposes. I'd rather not be maintaining it. I believe Mono have ditched this implementation since that time in favour of MS open source framework.

In addition, silent failures for unexpected behaviour is a bad thing. Throwing an exception when you're out of bounds is better than doing something unexpected.

I wont be accepting this change, although I would consider deleting this file.