Direct debug dialog hangs in OnSize()

When I build pgAdmin from sources on my laptop running Debian, and try
to open the Direct Debug dialog by choosing "Debug" on a function,
pgAdmin hangs, consuming 100% of CPU. The dialog never opens, and I have
no choice but kill pgAdmin forcibly. I only get this problem when I
build pgadmin from sources, the binary version from Debian repository
works fine.

I complained about this same issue years ago, and it was apparently
fixed back then
(http://archives.postgresql.org/pgadmin-hackers/2009-04/msg00017.php)
but looks like the issue has reappeared. Attaching a debugger to the
process when it's hung, I get a stack trace identical to that I posted
back in 2009. It's stuck repeatedly getting the OnSize event, and
resizing the window or some of the controls in response.

I don't understand much about this stuff, but I started comparing the
dlgDirectDbg window with dlgPgpassConfig window, which is very similar
but works. Looks to me there's some useless nested panels in the
dlgDirectDbg dialog, and removing them fixes the issue for me. After
that, the size of the window is wrong, though, so I added some extra
code to recalculate the desired window size after adding the arguments
to the grid in the dialog. I'm not sure what the original problem was or
why this fixes it, but the attached patch fixes it for me.

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com

Sorry, attached wrong patch in previous mail. Here's the correct one.

On 19.04.2012 23:42, Heikki Linnakangas wrote:
> When I build pgAdmin from sources on my laptop running Debian, and try
> to open the Direct Debug dialog by choosing "Debug" on a function,
> pgAdmin hangs, consuming 100% of CPU. The dialog never opens, and I have
> no choice but kill pgAdmin forcibly. I only get this problem when I
> build pgadmin from sources, the binary version from Debian repository
> works fine.
>
> I complained about this same issue years ago, and it was apparently
> fixed back then
> (http://archives.postgresql.org/pgadmin-hackers/2009-04/msg00017.php)
> but looks like the issue has reappeared. Attaching a debugger to the
> process when it's hung, I get a stack trace identical to that I posted
> back in 2009. It's stuck repeatedly getting the OnSize event, and
> resizing the window or some of the controls in response.
>
> I don't understand much about this stuff, but I started comparing the
> dlgDirectDbg window with dlgPgpassConfig window, which is very similar
> but works. Looks to me there's some useless nested panels in the
> dlgDirectDbg dialog, and removing them fixes the issue for me. After
> that, the size of the window is wrong, though, so I added some extra
> code to recalculate the desired window size after adding the arguments
> to the grid in the dialog. I'm not sure what the original problem was or
> why this fixes it, but the attached patch fixes it for me.
>

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com

And now that I've updated my local repository, here's a third version
that actually applies. (just my working copy didn't fix the issue)

On 19.04.2012 23:43, Heikki Linnakangas wrote:
> Sorry, attached wrong patch in previous mail. Here's the correct one.
>
> On 19.04.2012 23:42, Heikki Linnakangas wrote:
>> When I build pgAdmin from sources on my laptop running Debian, and try
>> to open the Direct Debug dialog by choosing "Debug" on a function,
>> pgAdmin hangs, consuming 100% of CPU. The dialog never opens, and I have
>> no choice but kill pgAdmin forcibly. I only get this problem when I
>> build pgadmin from sources, the binary version from Debian repository
>> works fine.
>>
>> I complained about this same issue years ago, and it was apparently
>> fixed back then
>> (http://archives.postgresql.org/pgadmin-hackers/2009-04/msg00017.php)
>> but looks like the issue has reappeared. Attaching a debugger to the
>> process when it's hung, I get a stack trace identical to that I posted
>> back in 2009. It's stuck repeatedly getting the OnSize event, and
>> resizing the window or some of the controls in response.
>>
>> I don't understand much about this stuff, but I started comparing the
>> dlgDirectDbg window with dlgPgpassConfig window, which is very similar
>> but works. Looks to me there's some useless nested panels in the
>> dlgDirectDbg dialog, and removing them fixes the issue for me. After
>> that, the size of the window is wrong, though, so I added some extra
>> code to recalculate the desired window size after adding the arguments
>> to the grid in the dialog. I'm not sure what the original problem was or
>> why this fixes it, but the attached patch fixes it for me.
>>
>
>

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com

Hi

On Thu, Apr 19, 2012 at 9:59 PM, Heikki Linnakangas
<heikki(dot)linnakangas(at)enterprisedb(dot)com> wrote:
> And now that I've updated my local repository, here's a third version that
> actually applies. (just my working copy didn't fix the issue)

With this change, the dialogue no longer meets our design standard.

Please see http://wiki.postgresql.org/wiki/PgAdmin_Internals#Property_page_dialogue_layout,
which should be the standard for all new/updated dialogues, except in
exceptional circumstances (with justification).

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

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

On Fri, 2012-04-20 at 09:21 +0100, Dave Page wrote:
> Hi
>
> On Thu, Apr 19, 2012 at 9:59 PM, Heikki Linnakangas
> <heikki(dot)linnakangas(at)enterprisedb(dot)com> wrote:
> > And now that I've updated my local repository, here's a third version that
> > actually applies. (just my working copy didn't fix the issue)
>
> With this change, the dialogue no longer meets our design standard.
>
> Please see http://wiki.postgresql.org/wiki/PgAdmin_Internals#Property_page_dialogue_layout,
> which should be the standard for all new/updated dialogues, except in
> exceptional circumstances (with justification).
>

I worked a bit on Heikki's patch yesterday evening and this morning. It
seems pgAdmin hangs when I want to debug functions with parameters. If
my function doesn't have any parameter, it works well.

IOW, it hangs when we call Show on the dlgDirectDeg dialog.

I don't understand why though. The XRC file seems good to me.

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

On Fri, 2012-04-20 at 10:47 +0200, Guillaume Lelarge wrote:
> On Fri, 2012-04-20 at 09:21 +0100, Dave Page wrote:
> > Hi
> >
> > On Thu, Apr 19, 2012 at 9:59 PM, Heikki Linnakangas
> > <heikki(dot)linnakangas(at)enterprisedb(dot)com> wrote:
> > > And now that I've updated my local repository, here's a third version that
> > > actually applies. (just my working copy didn't fix the issue)
> >
> > With this change, the dialogue no longer meets our design standard.
> >
> > Please see http://wiki.postgresql.org/wiki/PgAdmin_Internals#Property_page_dialogue_layout,
> > which should be the standard for all new/updated dialogues, except in
> > exceptional circumstances (with justification).
> >
>
> I worked a bit on Heikki's patch yesterday evening and this morning. It
> seems pgAdmin hangs when I want to debug functions with parameters. If
> my function doesn't have any parameter, it works well.
>
> IOW, it hangs when we call Show on the dlgDirectDeg dialog.
>
> I don't understand why though. The XRC file seems good to me.
>

I took a look at the thread Heikki mentionned in his first mail. One of
the issue he had was fixed by setting the size of the COL_NAME column.
We don't do that if we have to show the dialog. So I added the same
statement, and it works now. Kinda puzzling, but it works. And we still
meet our design standard. See attached patch.

Heikki, is this patch working for you?

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

On 20.04.2012 11:52, Guillaume Lelarge wrote:
> On Fri, 2012-04-20 at 10:47 +0200, Guillaume Lelarge wrote:
>> On Fri, 2012-04-20 at 09:21 +0100, Dave Page wrote:
>>> On Thu, Apr 19, 2012 at 9:59 PM, Heikki Linnakangas
>>> <heikki(dot)linnakangas(at)enterprisedb(dot)com> wrote:
>>>> And now that I've updated my local repository, here's a third version that
>>>> actually applies. (just my working copy didn't fix the issue)
>>>
>>> With this change, the dialogue no longer meets our design standard.
>>>
>>> Please see http://wiki.postgresql.org/wiki/PgAdmin_Internals#Property_page_dialogue_layout,
>>> which should be the standard for all new/updated dialogues, except in
>>> exceptional circumstances (with justification).

Ah, thanks for pointing that out.

> I took a look at the thread Heikki mentionned in his first mail. One of
> the issue he had was fixed by setting the size of the COL_NAME column.
> We don't do that if we have to show the dialog. So I added the same
> statement, and it works now. Kinda puzzling, but it works. And we still
> meet our design standard. See attached patch.
>
> Heikki, is this patch working for you?

Yes, that fixes it. Thanks!

While we're at it, can we make the 'value' column wider? It start really
small, and there's plenty of space in the grid.

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com

On Fri, 2012-04-20 at 13:26 +0300, Heikki Linnakangas wrote:
> On 20.04.2012 11:52, Guillaume Lelarge wrote:
> > On Fri, 2012-04-20 at 10:47 +0200, Guillaume Lelarge wrote:
> >> On Fri, 2012-04-20 at 09:21 +0100, Dave Page wrote:
> >>> On Thu, Apr 19, 2012 at 9:59 PM, Heikki Linnakangas
> >>> <heikki(dot)linnakangas(at)enterprisedb(dot)com> wrote:
> >>>> And now that I've updated my local repository, here's a third version that
> >>>> actually applies. (just my working copy didn't fix the issue)
> >>>
> >>> With this change, the dialogue no longer meets our design standard.
> >>>
> >>> Please see http://wiki.postgresql.org/wiki/PgAdmin_Internals#Property_page_dialogue_layout,
> >>> which should be the standard for all new/updated dialogues, except in
> >>> exceptional circumstances (with justification).
>
> Ah, thanks for pointing that out.
>
> > I took a look at the thread Heikki mentionned in his first mail. One of
> > the issue he had was fixed by setting the size of the COL_NAME column.
> > We don't do that if we have to show the dialog. So I added the same
> > statement, and it works now. Kinda puzzling, but it works. And we still
> > meet our design standard. See attached patch.
> >
> > Heikki, is this patch working for you?
>
> Yes, that fixes it. Thanks!
>
> While we're at it, can we make the 'value' column wider? It start really
> small, and there's plenty of space in the grid.
>

We now set the width of each column. The Type column and the Value
column are wider.

Commited and pushed.

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