[Stk] unhandled exception when using BlitSaw with std::vector

[Stephen Sinclair]

Hi Nick,

On Wed, Jun 19, 2013 at 3:09 PM, Nick Dika <nhdika at gmail.com> wrote:
> Thanks Stephen. Here's a simplified version of what I'm doing.
>
> This works:
>
> stk::BlitSaw oscArray[10];
>
> for( int i=0; i < 10; i++ )
> {
> stk::BlitSaw saw;
> saw.setHarmonics(0);
> saw.setFrequency(220);
> oscArray[i] = saw;
> }

Actually in g++-4.7 at least, this code does not compile.  The BlitSaw
class is unassignable.

(I'm not 100% sure why, but it seems to be because objects of class
Stk contain a std::ostringstream member which is unassignable.)


> This breaks:
>
> std::vector<stk::BlitSaw> oscs;
>
> for( int i=0; i < 10; i++ )
> {
> stk::BlitSaw saw;
> saw.setHarmonics(0);
> saw.setFrequency(220);
> oscs.push_back(saw);
> }

This is effectively equivalent to the above code so I'm not 100% why
it's breaking things but probably the object is not being properly
copied within the vector.  In particular, since you have a vector of
instances of BlitSaw, when the internal array in the std::vector is
re-allocated, perhaps the new, copied memory is not properly
initialized 100%, since the object type is not assignable.  Just a
guess, but it's weird behaviour for sure, but due to undefined
behaviour because this isn't how you should do what you're trying to
do.

>
> the second example causes an unhandled exception/access violation reading
> from a location. As mentioned this appears to happen after
> StkFrames::resize() is called (I don't fully understand why this would be
> called).

All Generators, which BlitSaw is one, have an StkFrame called
lastFrames_, and it's resize() method is called during initialization.
 Not sure why it's leading to an exception but like your array doesn't
contain valid BlitSaw instances.

Anyways, to elaborate, the way you're initializing things isn't
fundamentally wrong, so it could indicate some bugs somewhere, but
it's certainly not the ideal way to work with Stk objects.  You are
creating an array of objects, such that their constructors are called,
and then a loop you are constructing a new object on the stack,
setting its parameters, and then copying that object over top of the
objects already in the array and letting it destruct.  Not the most
efficient approach.

(Note, your push_back() call also doesn't compile in g++-4.7.)

Instead I'd suggest either directly initializing the objects that are
already in the array, or initializing them and then storing their
pointers in the array.  i.e., either:

#include <iostream>
#include <Stk.h>
#include <BlitSaw.h>

int main()
{
    stk::BlitSaw oscs[10];

    for( int i=0; i < 10; i++ )
    {
        stk::BlitSaw *saw = &oscs[i];
        saw->setHarmonics(0);
        saw->setFrequency(220);
    }

    return 0;
}


.. or ..


#include <iostream>
#include <memory>
#include <Stk.h>
#include <BlitSaw.h>

int main()
{
    std::vector<std::unique_ptr<stk::BlitSaw>> oscs;

    for( int i=0; i < 10; i++ )
    {
        std::unique_ptr<stk::BlitSaw> saw(new stk::BlitSaw);
        saw->setHarmonics(0);
        saw->setFrequency(220);
        oscs.push_back(std::move(saw));
    }

    return 0;
}


Steve