I am still working on a potential solution.
Mark
On 17 Sep 2016 16:52, "Paul Licameli" <***@gmail.com> wrote:
> I still favor reverting this:
>
> I found a simple way to reproduce this crash on Mac, either Debug or
> Release build. Does the same happen on other platforms? I expect it is
> likely.
>
> 1) Generate a long enough noise (I used 1 hour), so that step 2 will
> display the progress dialog.
> 2) Apply Normalize
>
> That's it.
>
> I took a closer look at this commit. It was done without regard to what
> ProgressDialog::SetMessage would do. Now that there are not one but two
> static text windows, something more complicated must be necessary here.
> The unchanged function is crashing because mMessage contains an
> unpredictable pointer value.
>
> I also see deletion of these lines, without explanation why:
> #if defined(__WXMAC__)
> mMessage->SetMinSize(wxSize(mLastW, mLastH));
> #endif
>
> Which were added by this commit:
>
> Author: Leland Lucius <***@homerow.net> 2015-05-22 11:14:22
> Committer: Leland Lucius <***@homerow.net> 2015-05-22 11:14:22
> Parent: a7207d2c4f4f845d4bf9ea321f32d6a2b3cecfef (Update Menus.cpp)
> Child: f9061e391612fd58fa849bdbe67326169dff40cf (Let's try Effect
> management once more)
> Branches: master, remotes/audacity/master and many more (80)
> Follows: Audacity-2.1.0
> Precedes: Audacity-2.1.1
> Corrects sizing on OSX
>
> Wouldn't you know that OSX had to go and be different, but it
> looks like we finally have consistency across all three (fingers
> crossed).
>
>
> No mention in the new commit's comments why it is all right to remove this.
>
> So there is more to do here than one or two lines to make the crash go
> away while honoring the intent of the change. Again I say, this change was
> not ready enough to be merged. I say revert it until something better is
> ready.
>
> PRL
>
>
> On Thu, Sep 15, 2016 at 3:18 PM, Steve the Fiddle <
> ***@gmail.com> wrote:
>
>> On 15 September 2016 at 19:14, Gale Andrews <***@audacityteam.org>
>> wrote:
>> > Thanks to Paul for pointing out the issue.
>> >
>> > Meantime would I assume Mark's commit has also caused much
>> > narrower progress dialogues generally, e.g. for Generate and
>> > effects?
>> >
>> > Do we want that, if so? I suppose it's a cosmetic judgement and
>> > depends on what you're used to.
>>
>> We now let WxWidgets handle the layout to fit the contents rather than
>> forcing a set size. This means that whether there is one column of
>> text or four, the dialog fits the text.
>>
>> Steve
>>
>> >
>> >
>> > Gale
>> >
>> >
>> > On 15 September 2016 at 16:49, Mark Young <***@tip2tail.scot> wrote:
>> >> I have an idea of how to fix this, but it will be later tonight before
>> I can
>> >> start on it if that's ok.
>> >>
>> >>
>> >> On 15 Sep 2016 16:18, "Paul Licameli" <***@gmail.com> wrote:
>> >>>
>> >>>
>> >>>
>> >>> On Thu, Sep 15, 2016 at 11:03 AM, Steve the Fiddle
>> >>> <***@gmail.com> wrote:
>> >>>>
>> >>>> On 15 September 2016 at 15:23, Paul Licameli <
>> ***@gmail.com>
>> >>>> wrote:
>> >>>> > If you mean a compiler error or warning, I don't think you will get
>> >>>> > one.
>> >>>>
>> >>>> No, I mean I don't get the repeatable crash that you predicted. I get
>> >>>> no warnings and no errors at all when running Timer Record or any
>> >>>> other progress dialog,
>> >>>
>> >>>
>> >>> There must be some repeatable way to reach ProgressDialog::Update
>> with a
>> >>> non-empty string argument, via Effect::TrackProgress. Try Nyquist
>> effects
>> >>> or analyzers.
>> >>>
>> >>>
>> >>>>
>> >>>>
>> >>>> > This problem of use of an uninitialized class member is probably
>> too
>> >>>> > subtle
>> >>>> > for it. The non-initialization is in one function, the use of the
>> >>>> > member is
>> >>>> > in another function. Not like an uninitialized local variable.
>> >>>> >
>> >>>> > I easily see from diffs Mark removed the code that initialized
>> mMessage
>> >>>> > with
>> >>>> > certain text, which wasn't trivial, and I don't know why that
>> change
>> >>>> > was
>> >>>> > necessary for the rest of it. The initialization should be put
>> back,
>> >>>> > but
>> >>>> > there is much else in the commit that I did not scrutinize and I
>> don't
>> >>>> > know
>> >>>> > how it all hangs together.
>> >>>> >
>> >>>> > PRL
>> >>>> >
>> >>>> >
>> >>>> > On Thu, Sep 15, 2016 at 9:52 AM, Steve the Fiddle
>> >>>> > <***@gmail.com>
>> >>>> > wrote:
>> >>>> >>
>> >>>> >> On 15 September 2016 at 14:05, Paul Licameli <
>> ***@gmail.com>
>> >>>> >> wrote:
>> >>>> >> > It happens because ProgressDialog::mMessage is a built in C
>> type, a
>> >>>> >> > bare
>> >>>> >> > pointer, and so gets an unpredictable value if it isn't
>> explicitly
>> >>>> >> > initialized. I think the progress dialog is the
>> stack-allocated one
>> >>>> >> > in
>> >>>> >> > Effect::DoEffect. So, left-over values from a previous
>> deepening of
>> >>>> >> > the
>> >>>> >> > program stack end up in the dialog, and that's how it can point
>> to a
>> >>>> >> > completely unrelated class.
>> >>>> >> >
>> >>>> >> > A simple thing to do would be to add an in-class initializer
>> (just
>> >>>> >> > {}
>> >>>> >> > after
>> >>>> >> > the member name).
>> >>>>
>> >>>> Do you mean, change line 124 of ProgressDialog.h to
>> >>>> wxStaticText *mMessage {};
>> >>>>
>> >>>> That's what I tried. No errors, it still works 100% reliably on
>> Linux.
>> >>>> Do you get a repeatable error or crash on any platform?
>> >>>
>> >>>
>> >>> See above. I haven't tried it, I'm just reasoning.
>> >>>
>> >>> PRL
>> >>>
>> >>>>
>> >>>>
>> >>>> Steve
>> >>>>
>> >>>> >>>That will make the pointer consistently NULL. That
>> >>>> >> > will
>> >>>> >> > not fix the bug but will make the crash repeatable on all
>> platforms,
>> >>>> >> > not
>> >>>> >> > Mac-only and intermittent as now.
>> >>>> >>
>> >>>> >> Good idea, but I've tried that, and still no error or warning on
>> >>>> >> Linux.
>> >>>> >>
>> >>>> >> Steve
>> >>>> >>
>> >>>> >> >
>> >>>> >> > I think this commit was not given a thorough enough review
>> before
>> >>>> >> > merging.
>> >>>> >> >
>> >>>> >> > PRL
>> >>>> >> >
>> >>>> >> >
>> >>>> >> > On Thu, Sep 15, 2016 at 7:35 AM, Steve the Fiddle
>> >>>> >> > <***@gmail.com>
>> >>>> >> > wrote:
>> >>>> >> >>
>> >>>> >> >> Paul wrote:
>> >>>> >> >> > The poiner mMessage does not point to a wxStaticText as it
>> >>>> >> >> > should,
>> >>>> >> >> > but
>> >>>> >> >> > somehow points to a wxClientDC object
>> >>>> >> >>
>> >>>> >> >> How can that happen when mMessage has been declared as
>> wxStaticText
>> >>>> >> >> *mMessage ?
>> >>>> >> >>
>> >>>> >> >> Given that there are no compiler warnings and no errors on
>> Linux,
>> >>>> >> >> how
>> >>>> >> >> can I test when it is fixed?
>> >>>> >> >>
>> >>>> >> >> Steve
>> >>>> >> >>
>> >>>> >> >> On 15 September 2016 at 12:01, Mark Young <***@tip2tail.scot>
>> >>>> >> >> wrote:
>> >>>> >> >> > Hi
>> >>>> >> >> >
>> >>>> >> >> > I will have a look at this later today.
>> >>>> >> >> >
>> >>>> >> >> > Thanks
>> >>>> >> >> >
>> >>>> >> >> > Mark
>> >>>> >> >> >
>> >>>> >> >> >
>> >>>> >> >> > On 15 Sep 2016 11:57 a.m., "Paul Licameli"
>> >>>> >> >> > <***@gmail.com>
>> >>>> >> >> > wrote:
>> >>>> >> >> >>
>> >>>> >> >> >> I did not study the commit enough to write the fix for
>> myself.
>> >>>> >> >> >> There
>> >>>> >> >> >> are
>> >>>> >> >> >> more than a few changes to the intialization function. I
>> don't
>> >>>> >> >> >> know
>> >>>> >> >> >> what
>> >>>> >> >> >> Mark meant by removal of the relevant lines.
>> >>>> >> >> >>
>> >>>> >> >> >> PRL
>> >>>> >> >> >>
>> >>>> >> >> >>
>> >>>> >> >> >> On Thu, Sep 15, 2016 at 6:47 AM, James Crook <
>> ***@indigo.ie>
>> >>>> >> >> >> wrote:
>> >>>> >> >> >>>
>> >>>> >> >> >>> +1 for fixing.
>> >>>> >> >> >>> Please would you fix it Steve.
>> >>>> >> >> >>>
>> >>>> >> >> >>>
>> >>>> >> >> >>>
>> >>>> >> >> >>> On 9/15/2016 8:23 AM, Steve the Fiddle wrote:
>> >>>> >> >> >>> > -1 for reverting.
>> >>>> >> >> >>> > +1 for fixing.
>> >>>> >> >> >>> >
>> >>>> >> >> >>> > Doesn't it just need mMessage to be initialised?
>> >>>> >> >> >>> >
>> >>>> >> >> >>> > Steve
>> >>>> >> >> >>> >
>> >>>> >> >> >>> > On 15 September 2016 at 05:57, Paul Licameli
>> >>>> >> >> >>> > <***@gmail.com>
>> >>>> >> >> >>> > wrote:
>> >>>> >> >> >>> >> This commit removed all code that initializes
>> >>>> >> >> >>> >> ProgressDialog::mMessage
>> >>>> >> >> >>> >> but
>> >>>> >> >> >>> >> did not remove the code that uses it, which therefore
>> >>>> >> >> >>> >> sometimes
>> >>>> >> >> >>> >> uses a
>> >>>> >> >> >>> >> random pointer.
>> >>>> >> >> >>> >>
>> >>>> >> >> >>> >> Details for reproducing the crash may be implementation
>> >>>> >> >> >>> >> dependent.
>> >>>> >> >> >>> >> On
>> >>>> >> >> >>> >> my
>> >>>> >> >> >>> >> Mac I ran into it when using the Beat Finder analyzer
>> when
>> >>>> >> >> >>> >> selecting
>> >>>> >> >> >>> >> all of
>> >>>> >> >> >>> >> a project that contains a Wave track and a Note track.
>> >>>> >> >> >>> >>
>> >>>> >> >> >>> >> Part of the stack trace looks like this:
>> >>>> >> >> >>> >>
>> >>>> >> >> >>> >> #10 0x01f5e8b4 in wxClientDC::wxClientDC(wxWindow*) ()
>> >>>> >> >> >>> >> #11 0x004adf16 in ProgressDialog::SetMessage(wxString
>> >>>> >> >> >>> >> const&) at
>> >>>> >> >> >>> >>
>> >>>> >> >> >>> >>
>> >>>> >> >> >>> >>
>> >>>> >> >> >>> >>
>> >>>> >> >> >>> >> /Users/paullicameli/GitHub/aud
>> acity/src/widgets/ProgressDialog.cpp:1441
>> >>>> >> >> >>> >> #12 0x004ad98b in ProgressDialog::Update(int, wxString
>> >>>> >> >> >>> >> const&)
>> >>>> >> >> >>> >> at
>> >>>> >> >> >>> >>
>> >>>> >> >> >>> >>
>> >>>> >> >> >>> >>
>> >>>> >> >> >>> >>
>> >>>> >> >> >>> >> /Users/paullicameli/GitHub/aud
>> acity/src/widgets/ProgressDialog.cpp:1303
>> >>>> >> >> >>> >> #13 0x004ae576 in ProgressDialog::Update(double, double,
>> >>>> >> >> >>> >> wxString
>> >>>> >> >> >>> >> const&) at
>> >>>> >> >> >>> >>
>> >>>> >> >> >>> >>
>> >>>> >> >> >>> >>
>> >>>> >> >> >>> >>
>> >>>> >> >> >>> >> /Users/paullicameli/GitHub/aud
>> acity/src/widgets/ProgressDialog.cpp:1423
>> >>>> >> >> >>> >> #14 0x000b0628 in Effect::TrackProgress(int, double,
>> >>>> >> >> >>> >> wxString
>> >>>> >> >> >>> >> const&)
>> >>>> >> >> >>> >> at
>> >>>> >> >> >>> >>
>> >>>> >> >> >>> >> /Users/paullicameli/GitHub/aud
>> acity/src/effects/Effect.cpp:1981
>> >>>> >> >> >>> >> #15 0x00115689 in NyquistEffect::ProcessOne() at
>> >>>> >> >> >>> >>
>> >>>> >> >> >>> >>
>> >>>> >> >> >>> >>
>> >>>> >> >> >>> >>
>> >>>> >> >> >>> >> /Users/paullicameli/GitHub/aud
>> acity/src/effects/nyquist/Nyquist.cpp:1079
>> >>>> >> >> >>> >> #16 0x0011217c in NyquistEffect::Process() at
>> >>>> >> >> >>> >>
>> >>>> >> >> >>> >>
>> >>>> >> >> >>> >>
>> >>>> >> >> >>> >>
>> >>>> >> >> >>> >> /Users/paullicameli/GitHub/aud
>> acity/src/effects/nyquist/Nyquist.cpp:702
>> >>>> >> >> >>> >>
>> >>>> >> >> >>> >> Where ProgressDialog::SetMessage calls
>> >>>> >> >> >>> >> mMessage->SetLabel(message);
>> >>>> >> >> >>> >> The poiner mMessage does not point to a wxStaticText as
>> it
>> >>>> >> >> >>> >> should,
>> >>>> >> >> >>> >> but
>> >>>> >> >> >>> >> somehow points to a wxClientDC object, and so virtual
>> >>>> >> >> >>> >> function
>> >>>> >> >> >>> >> dispatch goes
>> >>>> >> >> >>> >> crazy.
>> >>>> >> >> >>> >>
>> >>>> >> >> >>> >> This may explain crashes on Mac that Gale reported.
>> >>>> >> >> >>> >>
>> >>>> >> >> >>> >> I recommend reverting this commit, and Mark should fix
>> it
>> >>>> >> >> >>> >> and
>> >>>> >> >> >>> >> propose
>> >>>> >> >> >>> >> a new
>> >>>> >> >> >>> >> pull request.
>> >>>> >> >> >>> >>
>> >>>> >> >> >>> >> PRL
>> >>>> >> >> >>> >>
>> >>>> >> >> >>> >>
>> >>>> >> >> >>> >>
>> >>>> >> >> >>> >>
>> >>>> >> >> >>> >>
>> >>>> >> >> >>> >>
>> >>>> >> >> >>> >> ------------------------------
>> ------------------------------------------------
>> >>>> >> >> >>> >>
>> >>>> >> >> >>> >> _______________________________________________
>> >>>> >> >> >>> >> audacity-devel mailing list
>> >>>> >> >> >>> >> audacity-***@lists.sourceforge.net
>> >>>> >> >> >>> >> https://lists.sourceforge.net/
>> lists/listinfo/audacity-devel
>> >>>> >> >> >>> >>
>> >>>> >> >> >>> >
>> >>>> >> >> >>> >
>> >>>> >> >> >>> >
>> >>>> >> >> >>> >
>> >>>> >> >> >>> > ------------------------------
>> ------------------------------------------------
>> >>>> >> >> >>> > _______________________________________________
>> >>>> >> >> >>> > audacity-devel mailing list
>> >>>> >> >> >>> > audacity-***@lists.sourceforge.net
>> >>>> >> >> >>> > https://lists.sourceforge.net/
>> lists/listinfo/audacity-devel
>> >>>> >> >> >>> >
>> >>>> >> >> >>> >
>> >>>> >> >> >>>
>> >>>> >> >> >>>
>> >>>> >> >> >>>
>> >>>> >> >> >>>
>> >>>> >> >> >>>
>> >>>> >> >> >>>
>> >>>> >> >> >>> ------------------------------
>> ------------------------------------------------
>> >>>> >> >> >>> _______________________________________________
>> >>>> >> >> >>> audacity-devel mailing list
>> >>>> >> >> >>> audacity-***@lists.sourceforge.net
>> >>>> >> >> >>> https://lists.sourceforge.net/
>> lists/listinfo/audacity-devel
>> >>>> >> >> >>
>> >>>> >> >> >>
>> >>>> >> >> >>
>> >>>> >> >> >>
>> >>>> >> >> >>
>> >>>> >> >> >>
>> >>>> >> >> >>
>> >>>> >> >> >> ------------------------------
>> ------------------------------------------------
>> >>>> >> >> >>
>> >>>> >> >> >> _______________________________________________
>> >>>> >> >> >> audacity-devel mailing list
>> >>>> >> >> >> audacity-***@lists.sourceforge.net
>> >>>> >> >> >> https://lists.sourceforge.net/lists/listinfo/audacity-devel
>> >>>> >> >> >>
>> >>>> >> >> >
>> >>>> >> >> >
>> >>>> >> >> >
>> >>>> >> >> >
>> >>>> >> >> > ------------------------------------------------------------
>> ------------------
>> >>>> >> >> >
>> >>>> >> >> > _______________________________________________
>> >>>> >> >> > audacity-devel mailing list
>> >>>> >> >> > audacity-***@lists.sourceforge.net
>> >>>> >> >> > https://lists.sourceforge.net/lists/listinfo/audacity-devel
>> >>>> >> >> >
>> >>>> >> >>
>> >>>> >> >>
>> >>>> >> >>
>> >>>> >> >>
>> >>>> >> >> ------------------------------------------------------------
>> ------------------
>> >>>> >> >> _______________________________________________
>> >>>> >> >> audacity-devel mailing list
>> >>>> >> >> audacity-***@lists.sourceforge.net
>> >>>> >> >> https://lists.sourceforge.net/lists/listinfo/audacity-devel
>> >>>> >> >
>> >>>> >> >
>> >>>> >> >
>> >>>> >> >
>> >>>> >> >
>> >>>> >> > ------------------------------------------------------------
>> ------------------
>> >>>> >> >
>> >>>> >> > _______________________________________________
>> >>>> >> > audacity-devel mailing list
>> >>>> >> > audacity-***@lists.sourceforge.net
>> >>>> >> > https://lists.sourceforge.net/lists/listinfo/audacity-devel
>> >>>> >> >
>> >>>> >>
>> >>>> >>
>> >>>> >>
>> >>>> >> ------------------------------------------------------------
>> ------------------
>> >>>> >> _______________________________________________
>> >>>> >> audacity-devel mailing list
>> >>>> >> audacity-***@lists.sourceforge.net
>> >>>> >> https://lists.sourceforge.net/lists/listinfo/audacity-devel
>> >>>> >
>> >>>> >
>> >>>> >
>> >>>> >
>> >>>> > ------------------------------------------------------------
>> ------------------
>> >>>> >
>> >>>> > _______________________________________________
>> >>>> > audacity-devel mailing list
>> >>>> > audacity-***@lists.sourceforge.net
>> >>>> > https://lists.sourceforge.net/lists/listinfo/audacity-devel
>> >>>> >
>> >>>>
>> >>>>
>> >>>> ------------------------------------------------------------
>> ------------------
>> >>>> _______________________________________________
>> >>>> audacity-devel mailing list
>> >>>> audacity-***@lists.sourceforge.net
>> >>>> https://lists.sourceforge.net/lists/listinfo/audacity-devel
>> >>>
>> >>>
>> >>>
>> >>>
>> >>> ------------------------------------------------------------
>> ------------------
>> >>>
>> >>> _______________________________________________
>> >>> audacity-devel mailing list
>> >>> audacity-***@lists.sourceforge.net
>> >>> https://lists.sourceforge.net/lists/listinfo/audacity-devel
>> >>>
>> >>
>> >> ------------------------------------------------------------
>> ------------------
>> >>
>> >> _______________________________________________
>> >> audacity-devel mailing list
>> >> audacity-***@lists.sourceforge.net
>> >> https://lists.sourceforge.net/lists/listinfo/audacity-devel
>> >>
>> >
>> > ------------------------------------------------------------
>> ------------------
>> > _______________________________________________
>> > audacity-devel mailing list
>> > audacity-***@lists.sourceforge.net
>> > https://lists.sourceforge.net/lists/listinfo/audacity-devel
>>
>> ------------------------------------------------------------
>> ------------------
>> _______________________________________________
>> audacity-devel mailing list
>> audacity-***@lists.sourceforge.net
>> https://lists.sourceforge.net/lists/listinfo/audacity-devel
>>
>
>
> ------------------------------------------------------------
> ------------------
>
> _______________________________________________
> audacity-devel mailing list
> audacity-***@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/audacity-devel
>
>