sillsdev / libpalaso

Palaso Library: A set of .Net libraries useful for developers of Language Software.
MIT License
44 stars 50 forks source link

BetterLabel in Writing System Dialog throwing "Cannot access a disposed object" error #6

Closed hatton closed 8 years ago

hatton commented 10 years ago

Reported by Jonathan Coombs:

I did go ahead and create a sample project that illustrates the problem (attached): you can use the first "Set..." button as many times as you like, but the second button (which relies on a persistent _wsModel) will only work the first time you click it. It's simply the difference between this (which works)...

        IWritingSystemRepository store = AppWritingSys.WritingSystems;
        WritingSystemSetupModel wsModel = new WritingSystemSetupModel(store);
        wsModel.SetCurrentIndexFromRfc46464(textBoxWs.Text);
        using (var d = new WritingSystemSetupDialog(wsModel))
        {
            d.ShowDialog(wsModel.CurrentRFC4646);
            this.textBoxWs.Text = wsModel.CurrentRFC4646;

and this (which crashes on second run, once you click a WS):

        _store = _store ?? AppWritingSys.WritingSystems;
        _wsModel = _wsModel ?? new WritingSystemSetupModel(_store);
        using (var d = new WritingSystemSetupDialog(_wsModel))
        {
            d.ShowDialog(_wsModel.CurrentRFC4646);
            this.textBoxWs.Text = _wsModel.CurrentRFC4646;

On the Palaso end of things, these two BetterLabels in WritingSystemSetupView are ending up getting disposed too early:

    private Palaso.UI.WindowsForms.Widgets.BetterLabel _rfc4646;
    private Palaso.UI.WindowsForms.Widgets.BetterLabel _languageName;

It seems that either the WS dialog is damaging the passed-in _wsModel, or gets it partly unwired on closing the dialog the first time. (I'm suspicious of BindToModel(), and of LocalizationHelper.OnControlDisposed() but don't really know.) My workaround in Solid has been to make sure that I use the first approach above.

Also, while debugging I noticed some excessive event ping-ponging between methods in BetterLabel, which could be reduced by swapping these lines in BetterLabel_SizeChanged() to: _previousWidth = Width; DetermineHeight();


Jonathan also sent around a zip of a project that reproduces the problem. The core is here:

using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Data;
using System.Drawing;
using System.Linq;
using System.Text;
using System.Windows.Forms;

using Palaso.UI.WindowsForms.WritingSystems;
using Palaso.WritingSystems;

namespace TestPalaso
{
    public partial class FormTestWs : Form
    {
        public FormTestWs()
        {
            InitializeComponent();
        }

        IWritingSystemRepository _store;
        WritingSystemSetupModel _wsModel;

        //Works: Totally non-persistent, other than putting a string value into a textbox.
        private void buttonOpenWsDialog_Click(object sender, EventArgs e)
        {
            IWritingSystemRepository store = AppWritingSys.WritingSystems;
            WritingSystemSetupModel wsModel = new WritingSystemSetupModel(store);
            wsModel.SetCurrentIndexFromRfc46464(textBoxWs.Text);
            using (var d = new WritingSystemSetupDialog(wsModel))
            {
                d.ShowDialog(wsModel.CurrentRFC4646);
                this.textBoxWs.Text = wsModel.CurrentRFC4646;
            }

        }

        // Crashes on second launch, once you click on a WS:
        private void buttonOpenWsDialogPersistent_Click(object sender, EventArgs e)
        {
            _store = _store ?? AppWritingSys.WritingSystems;
            _wsModel = _wsModel ?? new WritingSystemSetupModel(_store);
            using (var d = new WritingSystemSetupDialog(_wsModel))
            {
                d.ShowDialog(_wsModel.CurrentRFC4646);
                string s = _wsModel.CurrentRFC4646;
                this.textBoxWs.Text = s;
                _wsModel.SetCurrentIndexFromRfc46464(s); //prob redundant
            }
        }

    }
}
pconstrictor commented 10 years ago

Test Project is here: https://www.sugarsync.com/pf/D342747_6399192_7776155

Thanks for adding the issue, John. While working on Solid some more, I realized that my first test project only demonstrated a simple text box (not 'bound'), and that solution apparently crashes because the WSMODEL IS persistent. Whereas Solid uses a WS control with Bind() and seems fine with a persistent _wsModel, but it crashes if the WSDIALOG IS NOT persistent. At least, that's what I think I've been experiencing. In both cases, the crash is due to those two BetterLabels getting disposed.

I'm including a more complete and clearly-labeled version of that test project--could you use that instead? Sorry I wasn't able to actually find the root cause.

Note that my workaround for Solid seems to trigger another crash later (a bit different, but still related to disposing), upon exiting the application (appended). This probably only happens in debug mode on a developer's machine, but still... Maybe I'm misunderstanding how Dispose() is supposed to work. (Workaround 2: I call my cleanup method early, in the form close event--not Dispose()--to beat Palaso to the punch.)

Hoping this helps, Jon

Disposed not explicitly called on Palaso.UI.WindowsForms.i18n.LocalizationHelper. Parent container name was '_moreButton'. at Palaso.UI.WindowsForms.i18n.LocalizationHelper..ctor(IContainer container) at Palaso.UI.WindowsForms.WritingSystems.WritingSystemSetupView.InitializeComponent()