Обсуждение: Re: [pgadmin-support] 1.14 beta 3 crashes in Query Browser

Поиск
Список
Период
Сортировка

Re: [pgadmin-support] 1.14 beta 3 crashes in Query Browser

От
Guillaume Lelarge
Дата:
On Sat, 2011-08-20 at 22:51 +0200, Guillaume Lelarge wrote:
> On Sat, 2011-08-20 at 17:45 +0200, Guillaume Lelarge wrote:
> > On Sat, 2011-08-20 at 11:07 -0400, Colin Beckingham wrote:
> > > Opensuse 11.4, kernel 3.0.
> > >
> > > I note that if I begin query construction in the graphical query
> > > designer and click into the text panel to make adjustments, adding for
> > > example a "distinct" clause and then attempting to go back to the
> > > graphical panel causes a crash.
> > >
> > > Of course trying to use distinct in this context is not reasonable,
> > > however the programme does not recover gracefully or prevent the user
> > > from making certain edits.
> > >
> > > The crash is quite severe. I have no access to the screen at all and the
> > > only way out is to CTRL+ALT+DEL, and wait for the timer to expire and
> > > log me out, and then log back in.
> >
> > I was able to reproduce your issue. I have no idea what's going on, but
> > I'm investigating this.
> >
> > Thank you for reporting it.
> >
>
> So, bug doesn't happen on Windows. I didn't check on Mac OS X, but it's
> present on Linux. The culprit commit is the one that changed the
> wxNotebook into a wxAUINotebook. Not sure yet how we'll fix this.
>

Seems we have a real issue here. Bug is you can't fire wxMessageBox or
alike when you're in the function fired by a
EVT_AUINOTEBOOK_PAGE_CHANGED. Only on Linux. Why? I have no idea. But
even the wxWidgets auidemo sample has the issue (I have the patch for
those who want to try).

So, my next idea was to remove the wxMessageBox from the function that
calls it, but I don't find a way to do that.

I'm afraid we'll have to replace the wxAuiNotebook with a wxNotebook.

Any objection? Dave especially, since it was your patch?
(http://git.postgresql.org/gitweb/?p=pgadmin3.git;a=commit;h=41545a4aa159a7a579b7c97ba73a605db34453b7)


--
Guillaume
  http://blog.guillaume.lelarge.info
  http://www.dalibo.com


Re: [pgadmin-support] 1.14 beta 3 crashes in Query Browser

От
Guillaume Lelarge
Дата:
On Mon, 2011-08-22 at 21:59 +0200, Guillaume Lelarge wrote:
> On Sat, 2011-08-20 at 22:51 +0200, Guillaume Lelarge wrote:
> > On Sat, 2011-08-20 at 17:45 +0200, Guillaume Lelarge wrote:
> > > On Sat, 2011-08-20 at 11:07 -0400, Colin Beckingham wrote:
> > > > Opensuse 11.4, kernel 3.0.
> > > >
> > > > I note that if I begin query construction in the graphical query
> > > > designer and click into the text panel to make adjustments, adding for
> > > > example a "distinct" clause and then attempting to go back to the
> > > > graphical panel causes a crash.
> > > >
> > > > Of course trying to use distinct in this context is not reasonable,
> > > > however the programme does not recover gracefully or prevent the user
> > > > from making certain edits.
> > > >
> > > > The crash is quite severe. I have no access to the screen at all and the
> > > > only way out is to CTRL+ALT+DEL, and wait for the timer to expire and
> > > > log me out, and then log back in.
> > >
> > > I was able to reproduce your issue. I have no idea what's going on, but
> > > I'm investigating this.
> > >
> > > Thank you for reporting it.
> > >
> >
> > So, bug doesn't happen on Windows. I didn't check on Mac OS X, but it's
> > present on Linux. The culprit commit is the one that changed the
> > wxNotebook into a wxAUINotebook. Not sure yet how we'll fix this.
> >
>
> Seems we have a real issue here. Bug is you can't fire wxMessageBox or
> alike when you're in the function fired by a
> EVT_AUINOTEBOOK_PAGE_CHANGED. Only on Linux. Why? I have no idea. But
> even the wxWidgets auidemo sample has the issue (I have the patch for
> those who want to try).
>
> So, my next idea was to remove the wxMessageBox from the function that
> calls it, but I don't find a way to do that.
>
> I'm afraid we'll have to replace the wxAuiNotebook with a wxNotebook.
>
> Any objection? Dave especially, since it was your patch?
> (http://git.postgresql.org/gitweb/?p=pgadmin3.git;a=commit;h=41545a4aa159a7a579b7c97ba73a605db34453b7)
>

Seems I have a better fix (not mine, see
http://forums.wxwidgets.org/viewtopic.php?f=23&t=31090 for details):

diff --git a/pgadmin/frm/frmQuery.cpp b/pgadmin/frm/frmQuery.cpp
index 40b01dd..f195307 100644
--- a/pgadmin/frm/frmQuery.cpp
+++ b/pgadmin/frm/frmQuery.cpp
@@ -2101,6 +2101,10 @@ bool frmQuery::updateFromGqb(bool executing)
                else
                        fn = _("The generated SQL query has changed.\nDo
you want to update it?");

+        wxWindow* win = wxWindow::GetCapture();
+        if (win)
+            win->ReleaseMouse();
+
                wxMessageDialog msg(this, fn, _("Query"), wxYES_NO |
wxICON_EXCLAMATION);
                if(msg.ShowModal() == wxID_YES && changed)
                {

Works on Linux. Will try on Windows tonight, before applying.


--
Guillaume
  http://blog.guillaume.lelarge.info
  http://www.dalibo.com


Re: [pgadmin-support] 1.14 beta 3 crashes in Query Browser

От
Dave Page
Дата:
On Tue, Aug 23, 2011 at 2:56 PM, Guillaume Lelarge
<guillaume@lelarge.info> wrote:
> On Mon, 2011-08-22 at 21:59 +0200, Guillaume Lelarge wrote:
>> On Sat, 2011-08-20 at 22:51 +0200, Guillaume Lelarge wrote:
>> > On Sat, 2011-08-20 at 17:45 +0200, Guillaume Lelarge wrote:
>> > > On Sat, 2011-08-20 at 11:07 -0400, Colin Beckingham wrote:
>> > > > Opensuse 11.4, kernel 3.0.
>> > > >
>> > > > I note that if I begin query construction in the graphical query
>> > > > designer and click into the text panel to make adjustments, adding for
>> > > > example a "distinct" clause and then attempting to go back to the
>> > > > graphical panel causes a crash.
>> > > >
>> > > > Of course trying to use distinct in this context is not reasonable,
>> > > > however the programme does not recover gracefully or prevent the user
>> > > > from making certain edits.
>> > > >
>> > > > The crash is quite severe. I have no access to the screen at all and the
>> > > > only way out is to CTRL+ALT+DEL, and wait for the timer to expire and
>> > > > log me out, and then log back in.
>> > >
>> > > I was able to reproduce your issue. I have no idea what's going on, but
>> > > I'm investigating this.
>> > >
>> > > Thank you for reporting it.
>> > >
>> >
>> > So, bug doesn't happen on Windows. I didn't check on Mac OS X, but it's
>> > present on Linux. The culprit commit is the one that changed the
>> > wxNotebook into a wxAUINotebook. Not sure yet how we'll fix this.
>> >
>>
>> Seems we have a real issue here. Bug is you can't fire wxMessageBox or
>> alike when you're in the function fired by a
>> EVT_AUINOTEBOOK_PAGE_CHANGED. Only on Linux. Why? I have no idea. But
>> even the wxWidgets auidemo sample has the issue (I have the patch for
>> those who want to try).
>>
>> So, my next idea was to remove the wxMessageBox from the function that
>> calls it, but I don't find a way to do that.
>>
>> I'm afraid we'll have to replace the wxAuiNotebook with a wxNotebook.
>>
>> Any objection? Dave especially, since it was your patch?
>> (http://git.postgresql.org/gitweb/?p=pgadmin3.git;a=commit;h=41545a4aa159a7a579b7c97ba73a605db34453b7)
>>
>
> Seems I have a better fix (not mine, see
> http://forums.wxwidgets.org/viewtopic.php?f=23&t=31090 for details):
>
> diff --git a/pgadmin/frm/frmQuery.cpp b/pgadmin/frm/frmQuery.cpp
> index 40b01dd..f195307 100644
> --- a/pgadmin/frm/frmQuery.cpp
> +++ b/pgadmin/frm/frmQuery.cpp
> @@ -2101,6 +2101,10 @@ bool frmQuery::updateFromGqb(bool executing)
>                else
>                        fn = _("The generated SQL query has changed.\nDo
> you want to update it?");
>
> +        wxWindow* win = wxWindow::GetCapture();
> +        if (win)
> +            win->ReleaseMouse();
> +
>                wxMessageDialog msg(this, fn, _("Query"), wxYES_NO |
> wxICON_EXCLAMATION);
>                if(msg.ShowModal() == wxID_YES && changed)
>                {
>
> Works on Linux. Will try on Windows tonight, before applying.

OK, cool. Believe it or not, I was just running a clean build to start
testing this myself when your email arrived :-)


--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Re: [pgadmin-support] 1.14 beta 3 crashes in Query Browser

От
Guillaume Lelarge
Дата:
On Tue, 2011-08-23 at 15:08 +0100, Dave Page wrote:
> On Tue, Aug 23, 2011 at 2:56 PM, Guillaume Lelarge
> <guillaume@lelarge.info> wrote:
> > On Mon, 2011-08-22 at 21:59 +0200, Guillaume Lelarge wrote:
> >> On Sat, 2011-08-20 at 22:51 +0200, Guillaume Lelarge wrote:
> >> > On Sat, 2011-08-20 at 17:45 +0200, Guillaume Lelarge wrote:
> >> > > On Sat, 2011-08-20 at 11:07 -0400, Colin Beckingham wrote:
> >> > > > Opensuse 11.4, kernel 3.0.
> >> > > >
> >> > > > I note that if I begin query construction in the graphical query
> >> > > > designer and click into the text panel to make adjustments, adding for
> >> > > > example a "distinct" clause and then attempting to go back to the
> >> > > > graphical panel causes a crash.
> >> > > >
> >> > > > Of course trying to use distinct in this context is not reasonable,
> >> > > > however the programme does not recover gracefully or prevent the user
> >> > > > from making certain edits.
> >> > > >
> >> > > > The crash is quite severe. I have no access to the screen at all and the
> >> > > > only way out is to CTRL+ALT+DEL, and wait for the timer to expire and
> >> > > > log me out, and then log back in.
> >> > >
> >> > > I was able to reproduce your issue. I have no idea what's going on, but
> >> > > I'm investigating this.
> >> > >
> >> > > Thank you for reporting it.
> >> > >
> >> >
> >> > So, bug doesn't happen on Windows. I didn't check on Mac OS X, but it's
> >> > present on Linux. The culprit commit is the one that changed the
> >> > wxNotebook into a wxAUINotebook. Not sure yet how we'll fix this.
> >> >
> >>
> >> Seems we have a real issue here. Bug is you can't fire wxMessageBox or
> >> alike when you're in the function fired by a
> >> EVT_AUINOTEBOOK_PAGE_CHANGED. Only on Linux. Why? I have no idea. But
> >> even the wxWidgets auidemo sample has the issue (I have the patch for
> >> those who want to try).
> >>
> >> So, my next idea was to remove the wxMessageBox from the function that
> >> calls it, but I don't find a way to do that.
> >>
> >> I'm afraid we'll have to replace the wxAuiNotebook with a wxNotebook.
> >>
> >> Any objection? Dave especially, since it was your patch?
> >> (http://git.postgresql.org/gitweb/?p=pgadmin3.git;a=commit;h=41545a4aa159a7a579b7c97ba73a605db34453b7)
> >>
> >
> > Seems I have a better fix (not mine, see
> > http://forums.wxwidgets.org/viewtopic.php?f=23&t=31090 for details):
> >
> > diff --git a/pgadmin/frm/frmQuery.cpp b/pgadmin/frm/frmQuery.cpp
> > index 40b01dd..f195307 100644
> > --- a/pgadmin/frm/frmQuery.cpp
> > +++ b/pgadmin/frm/frmQuery.cpp
> > @@ -2101,6 +2101,10 @@ bool frmQuery::updateFromGqb(bool executing)
> >                else
> >                        fn = _("The generated SQL query has changed.\nDo
> > you want to update it?");
> >
> > +        wxWindow* win = wxWindow::GetCapture();
> > +        if (win)
> > +            win->ReleaseMouse();
> > +
> >                wxMessageDialog msg(this, fn, _("Query"), wxYES_NO |
> > wxICON_EXCLAMATION);
> >                if(msg.ShowModal() == wxID_YES && changed)
> >                {
> >
> > Works on Linux. Will try on Windows tonight, before applying.
>
> OK, cool. Believe it or not, I was just running a clean build to start
> testing this myself when your email arrived :-)
>

:)

If you understand the fix, and can explain it to me, that would be
great. Right now, I don't get it (and don't have much time to get into
this).


--
Guillaume
  http://blog.guillaume.lelarge.info
  http://www.dalibo.com


Re: [pgadmin-support] 1.14 beta 3 crashes in Query Browser

От
Dave Page
Дата:
On Tue, Aug 23, 2011 at 3:16 PM, Guillaume Lelarge
<guillaume@lelarge.info> wrote:
> On Tue, 2011-08-23 at 15:08 +0100, Dave Page wrote:
>> On Tue, Aug 23, 2011 at 2:56 PM, Guillaume Lelarge
>> <guillaume@lelarge.info> wrote:
>> > On Mon, 2011-08-22 at 21:59 +0200, Guillaume Lelarge wrote:
>> >> On Sat, 2011-08-20 at 22:51 +0200, Guillaume Lelarge wrote:
>> >> > On Sat, 2011-08-20 at 17:45 +0200, Guillaume Lelarge wrote:
>> >> > > On Sat, 2011-08-20 at 11:07 -0400, Colin Beckingham wrote:
>> >> > > > Opensuse 11.4, kernel 3.0.
>> >> > > >
>> >> > > > I note that if I begin query construction in the graphical query
>> >> > > > designer and click into the text panel to make adjustments, adding for
>> >> > > > example a "distinct" clause and then attempting to go back to the
>> >> > > > graphical panel causes a crash.
>> >> > > >
>> >> > > > Of course trying to use distinct in this context is not reasonable,
>> >> > > > however the programme does not recover gracefully or prevent the user
>> >> > > > from making certain edits.
>> >> > > >
>> >> > > > The crash is quite severe. I have no access to the screen at all and the
>> >> > > > only way out is to CTRL+ALT+DEL, and wait for the timer to expire and
>> >> > > > log me out, and then log back in.
>> >> > >
>> >> > > I was able to reproduce your issue. I have no idea what's going on, but
>> >> > > I'm investigating this.
>> >> > >
>> >> > > Thank you for reporting it.
>> >> > >
>> >> >
>> >> > So, bug doesn't happen on Windows. I didn't check on Mac OS X, but it's
>> >> > present on Linux. The culprit commit is the one that changed the
>> >> > wxNotebook into a wxAUINotebook. Not sure yet how we'll fix this.
>> >> >
>> >>
>> >> Seems we have a real issue here. Bug is you can't fire wxMessageBox or
>> >> alike when you're in the function fired by a
>> >> EVT_AUINOTEBOOK_PAGE_CHANGED. Only on Linux. Why? I have no idea. But
>> >> even the wxWidgets auidemo sample has the issue (I have the patch for
>> >> those who want to try).
>> >>
>> >> So, my next idea was to remove the wxMessageBox from the function that
>> >> calls it, but I don't find a way to do that.
>> >>
>> >> I'm afraid we'll have to replace the wxAuiNotebook with a wxNotebook.
>> >>
>> >> Any objection? Dave especially, since it was your patch?
>> >> (http://git.postgresql.org/gitweb/?p=pgadmin3.git;a=commit;h=41545a4aa159a7a579b7c97ba73a605db34453b7)
>> >>
>> >
>> > Seems I have a better fix (not mine, see
>> > http://forums.wxwidgets.org/viewtopic.php?f=23&t=31090 for details):
>> >
>> > diff --git a/pgadmin/frm/frmQuery.cpp b/pgadmin/frm/frmQuery.cpp
>> > index 40b01dd..f195307 100644
>> > --- a/pgadmin/frm/frmQuery.cpp
>> > +++ b/pgadmin/frm/frmQuery.cpp
>> > @@ -2101,6 +2101,10 @@ bool frmQuery::updateFromGqb(bool executing)
>> >                else
>> >                        fn = _("The generated SQL query has changed.\nDo
>> > you want to update it?");
>> >
>> > +        wxWindow* win = wxWindow::GetCapture();
>> > +        if (win)
>> > +            win->ReleaseMouse();
>> > +
>> >                wxMessageDialog msg(this, fn, _("Query"), wxYES_NO |
>> > wxICON_EXCLAMATION);
>> >                if(msg.ShowModal() == wxID_YES && changed)
>> >                {
>> >
>> > Works on Linux. Will try on Windows tonight, before applying.
>>
>> OK, cool. Believe it or not, I was just running a clean build to start
>> testing this myself when your email arrived :-)
>>
>
> :)
>
> If you understand the fix, and can explain it to me, that would be
> great. Right now, I don't get it (and don't have much time to get into
> this).

Not entirely sure, but I think the tab control is capturing the mouse
focus for the duration of the event. What the fix does is to get
whatever wxWindow currently has capture, and tells it to release the
mouse, thus allowing the message box to receive the input.



--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Re: [pgadmin-support] 1.14 beta 3 crashes in Query Browser

От
Guillaume Lelarge
Дата:
On Tue, 2011-08-23 at 16:06 +0100, Dave Page wrote:
> On Tue, Aug 23, 2011 at 3:16 PM, Guillaume Lelarge
> <guillaume@lelarge.info> wrote:
> > On Tue, 2011-08-23 at 15:08 +0100, Dave Page wrote:
> >> On Tue, Aug 23, 2011 at 2:56 PM, Guillaume Lelarge
> >> <guillaume@lelarge.info> wrote:
> >> > On Mon, 2011-08-22 at 21:59 +0200, Guillaume Lelarge wrote:
> >> >> On Sat, 2011-08-20 at 22:51 +0200, Guillaume Lelarge wrote:
> >> >> > On Sat, 2011-08-20 at 17:45 +0200, Guillaume Lelarge wrote:
> >> >> > > On Sat, 2011-08-20 at 11:07 -0400, Colin Beckingham wrote:
> >> >> > > > Opensuse 11.4, kernel 3.0.
> >> >> > > >
> >> >> > > > I note that if I begin query construction in the graphical query
> >> >> > > > designer and click into the text panel to make adjustments, adding for
> >> >> > > > example a "distinct" clause and then attempting to go back to the
> >> >> > > > graphical panel causes a crash.
> >> >> > > >
> >> >> > > > Of course trying to use distinct in this context is not reasonable,
> >> >> > > > however the programme does not recover gracefully or prevent the user
> >> >> > > > from making certain edits.
> >> >> > > >
> >> >> > > > The crash is quite severe. I have no access to the screen at all and the
> >> >> > > > only way out is to CTRL+ALT+DEL, and wait for the timer to expire and
> >> >> > > > log me out, and then log back in.
> >> >> > >
> >> >> > > I was able to reproduce your issue. I have no idea what's going on, but
> >> >> > > I'm investigating this.
> >> >> > >
> >> >> > > Thank you for reporting it.
> >> >> > >
> >> >> >
> >> >> > So, bug doesn't happen on Windows. I didn't check on Mac OS X, but it's
> >> >> > present on Linux. The culprit commit is the one that changed the
> >> >> > wxNotebook into a wxAUINotebook. Not sure yet how we'll fix this.
> >> >> >
> >> >>
> >> >> Seems we have a real issue here. Bug is you can't fire wxMessageBox or
> >> >> alike when you're in the function fired by a
> >> >> EVT_AUINOTEBOOK_PAGE_CHANGED. Only on Linux. Why? I have no idea. But
> >> >> even the wxWidgets auidemo sample has the issue (I have the patch for
> >> >> those who want to try).
> >> >>
> >> >> So, my next idea was to remove the wxMessageBox from the function that
> >> >> calls it, but I don't find a way to do that.
> >> >>
> >> >> I'm afraid we'll have to replace the wxAuiNotebook with a wxNotebook.
> >> >>
> >> >> Any objection? Dave especially, since it was your patch?
> >> >> (http://git.postgresql.org/gitweb/?p=pgadmin3.git;a=commit;h=41545a4aa159a7a579b7c97ba73a605db34453b7)
> >> >>
> >> >
> >> > Seems I have a better fix (not mine, see
> >> > http://forums.wxwidgets.org/viewtopic.php?f=23&t=31090 for details):
> >> >
> >> > diff --git a/pgadmin/frm/frmQuery.cpp b/pgadmin/frm/frmQuery.cpp
> >> > index 40b01dd..f195307 100644
> >> > --- a/pgadmin/frm/frmQuery.cpp
> >> > +++ b/pgadmin/frm/frmQuery.cpp
> >> > @@ -2101,6 +2101,10 @@ bool frmQuery::updateFromGqb(bool executing)
> >> >                else
> >> >                        fn = _("The generated SQL query has changed.\nDo
> >> > you want to update it?");
> >> >
> >> > +        wxWindow* win = wxWindow::GetCapture();
> >> > +        if (win)
> >> > +            win->ReleaseMouse();
> >> > +
> >> >                wxMessageDialog msg(this, fn, _("Query"), wxYES_NO |
> >> > wxICON_EXCLAMATION);
> >> >                if(msg.ShowModal() == wxID_YES && changed)
> >> >                {
> >> >
> >> > Works on Linux. Will try on Windows tonight, before applying.
> >>
> >> OK, cool. Believe it or not, I was just running a clean build to start
> >> testing this myself when your email arrived :-)
> >>
> >
> > :)
> >
> > If you understand the fix, and can explain it to me, that would be
> > great. Right now, I don't get it (and don't have much time to get into
> > this).
>
> Not entirely sure, but I think the tab control is capturing the mouse
> focus for the duration of the event. What the fix does is to get
> whatever wxWindow currently has capture, and tells it to release the
> mouse, thus allowing the message box to receive the input.
>

Yeah, that's also the explanation of one of the moderator of the
wxWidgets forum.

I tried the patch on Windows and it worked flawlessly.

So, I applied it. Quite glad it's fixed :)


--
Guillaume
  http://blog.guillaume.lelarge.info
  http://www.dalibo.com