Subject: Re: Stability and compatability issues on windows xp

From: Nikolai Zhubr <[email-masked]>
Date: Sat, 26 Sep 2015 22:32:24 +0300

Hi,
26.09.2015 21:41, melvin6925 write:
>
> Not sure if it helps, but I have pgadmin 1.18 running fine on win xp.
> Have you tried reinstalling it?
>

Just tried also 1.18.0, in hope it could be better.

Nope. The same CPU usage/responsiveness issue, although it seemingly 
takes some more time to trigger.

Playing a bit more with it, I have a feeling that the reason for 
misbehavour are these shiny new gradient tabs... Because depending on 
how I place open windows (hiding or unhiding some of them), the issue 
seems to appear and disappear. So in the end it is probably not a 
pgamdin's fault, but this does not automatically make me happy yet...

Thank you,
Nikolai

-- 
Sent via pgadmin-support mailing list ([email-masked])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-support

---

From: Nikolai Zhubr <[email-masked]>
Date: Sun, 27 Sep 2015 00:34:18 +0300

Hi again,

and still again regarding this, as I have just discovered that 1.22 beta 
is probably to be out soon (by looking through last git commits), maybe 
it is a good time to test and even fix something?

So, is it planned e.g. to publish some beta windows binaries?

If not, would it make sense to go into trouble of finding visual studio 
somewhere and compiling myself? Would anyone be able to assist me so as 
to try to get next release really usable on windows?

(Compiling C projects on windows can be quite some pain, therefore I'm 
rather reluctant to even try unless I can count on some informed advice)

Thank you,
Nikolai

26.09.2015 22:32, I wrote:
[...]

---

From: "J.F. Oster" <[email-masked]>
Date: Sun, 27 Sep 2015 08:20:19 +0300

Hello,

NZ> So, is it planned e.g. to publish some beta windows binaries?

+1 for publishing beta binaries.

NZ> I'm on windows xp sp3 (32-bit).

Windows XP is not supported officially, http://www.pgadmin.org/download/windows.php
  From v1.16.0, Windows XP/2003 or later is required, and from 1.20.0, Windows Vista/2008 is required.

But there was a commit related to XP after 1.20 release:
http://git.postgresql.org/gitweb/?p=pgadmin3.git;a=commit;h=b845880cc1352820b14a1e9af7b56eb3a513a38b
So probably it is supposed to work :)

NZ> If not, would it make sense to go into trouble of finding visual studio
NZ> somewhere and compiling myself? Would anyone be able to assist me so as
NZ> to try to get next release really usable on windows?

Building on Windows is somewhat tricky. I can assist but note that I
build on Windows 7 and have never seen the bug you described.

NZ> So I'm wondering, is it supposed that windows xp has to be immediately
NZ> abandoned, or maybe nobody is much interested in testing/using on 
NZ> windows platform altogether anymore so issues on windows just do not get 
NZ> discovered?

I think there are numerous pgAdmin III users on Windows 7.

-- 
Best regards,
 J.F.

---

From: Nikolai Zhubr <[email-masked]>
Date: Sun, 27 Sep 2015 14:12:50 +0300

Hi,
>    From v1.16.0, Windows XP/2003 or later is required, and from 1.20.0, Windows Vista/2008 is required.

Indeed, thanks, I've somehow overlooked this one.
And its a real pity (for me at least).

I wouldn't dismiss windows xp here just for that, and I can't imagine 
any real new feature of windows vista that pgadmin needs so much now.

Rather, it is probably that officially preferred version of microsoft 
build tools has dropped support for windows xp target (which is 
certainly logical from microsoft pov) and maintainers might have not 
enough resources and/or motivation to deal with a number of various 
microsoft compilers simultaneously (which is actually logical too).

And this leads to a somewhat ridiculous situation, that I've just easily 
updated postgresql from 9.0 to 9.4 here, all my apps are running fine, 
except that an (important, but still) auxilary productivity utility 
(which saves me lots of typing and is nice, yes) now wants me to change 
an operating system.

On the other hand, AFAIK wxwidgets is _just_ dropping support for win9x 
(!) now, and furthermore, the 2.8 version of wxwidgets used in 
officially released pgadmin 1.20 is somewhat antique (and AFAICS cannot 
be officially obtained other than compiled from source) so I'm not sure 
one could expect any fixes for it even if one needed some.

> So probably it is supposed to work :)

Ah, this change was applied after 1.20 release, so yes, despite 
officially not supported, it might happen that a binary valid for 
windows xp will also appear in an upcoming release.

> build on Windows 7 and have never seen the bug you described.

Ok, thanks. I'd suppose build process on windows should not depend much 
on the build host version.

Some more about the bug itself.
It is triggered easily in the SQL-query panel by just some typing, then 
clicking back and forth between the editor and tabs below, and some 
moving this panel window as a whole. In order to notice the problem 
quickly, I just fire taskmanager before and minimize it so I can see CPU 
load increase immediately.

The bug looks like a (in)famous invalidate-repaint-invalidate dead loop. 
That is: CPU load increases to the max, GUI responsiveness drops, window 
contents gradually become corrupt (not properly repainted), finally the 
window becomes completely dead and can not even be closed by the 'close' 
button. Or sometimes the window might come back to life subsequently, 
depending on how exactly I move and click it. If connected via remote 
desktop, visual artifacts get much more noticable.

Generally such problem (on windows) typically happens when an 
application tries to automatically rearrange window elements as a 
reaction so some GUI event (like e.g. window resize by user) and while 
doing such rearrangement it ocasionally (maybe indirectly) fires some 
other GUI event (of the same or another type), which in turn causes yet 
another rearrangement and so on, sometimes with visible flickering, 
sometimes without. (Seen this plenty.)

Now starting with vista, if I understand it correctly, window 
invalidation and repainting technique has changed somewhat, so, what is 
relevant to the issue in question, less invalidation events are exposed 
to an application (as they are now processed by window manager or 
whatever). While probably a reasonable change, it certainly doesn't drop 
a requirement that repainting must not cause cyclic invalidation. But if 
all of develpoment and testing is performed on windows 7 then it is 
perfectly clear the bug could well stay unnoticed.

I think it would be good to fix it. It might be even not so difficult. I 
could maybe provide remote access to a VM with windows xp if some 
developer is interested.
Besides, I suspect that with some luck the problem might also trigger on 
vista+ when running pgadmin through a remote desktop, although I'm not 
quite sure.

Thank you,
Nikolai

-- 
Sent via pgadmin-hackers mailing list ([email-masked])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers

---

From: Nikolai Zhubr <[email-masked]>
Date: Tue, 29 Sep 2015 01:17:05 +0300

Hi,
(and sorry for cross-posting, the issue is imo pretty important)

27.09.2015 14:12, I wrote:
[...]
> vista+ when running pgadmin through a remote desktop, although I'm not

Ok, no need for remote desktop even.
Just reproduced this on windows 7 64-bit easily, although visual effect 
is not so striking as on windows xp (probably due to smarter resource 
management in newer OS and/or faster computer).
And I've got much more details now.

This is with pgamin 1.20 from official enterprisedb's installer.

Here is how to reproduce. (Procedure valid at least for win xp and 7)
In under 1 minute!

- Aero have to be disabled (on versions that have it). If you can not or 
do not want to disable aero for some reason, you might try connecting 
via remote desktop instead, it should basically behave very similar, 
I've tested it as well.
- start windows task manager and minimize it so it just shows CPU load 
within a tray icon. (taskmgr.exe)
- start pgamin as usual, connect to some server, then click on some 
database, so that SQL-panel button becomes available.
- open SQL-panel (click the above mentioned button), then move SQL-panel 
window in the righthand direction till some part of SQL-editor window 
rectangle goes beyond the right border of the screen. Let is stay like 
that (partly invisible). Note that I mean exactly SQL-editor window 
itself, not additional notepad window that might happen to be located to 
the right of SQL-editor. (Or you can just close this notepad to be sure)
- now click into SQL-editor so that you can start typing.
- type just exactly this: 'select now()' then stop, do not touch 
anything and watch the CPU load. it should go 100% (50% on dual core, 
25% on quad core) and stick there.
- note: it is critical to put brackets there, otherwise bug is not 
triggered, so I'd guess this is related to code highlighting.
- now go to SQL-panel's main menu - select file - select exit. instead 
of closing, SQL-panel goes completely unresponsive here.

I'd be glad to hear some feedback (from windows users) :)

Thank you,
Nikolai

---

From: Nikolai Zhubr <[email-masked]>
Date: Tue, 29 Sep 2015 21:40:52 +0300

Hi,
29.09.2015 1:17, I wrote:
[...]
> Here is how to reproduce. (Procedure valid at least for win xp and 7)

Some more observations. Same problem happens also in function properties 
window, in the 'Code' tab:

- choose some simple (lets say empty) function, open its properties;
- click on the 'Code' tab;
- place this properties window so that its righthand margin is somewhat 
beyond the right margin of the monitor, making some part of SQL editor 
invisible (out of the real screen space);
- click to place cursor into the function body and type 'select now()';
- watch the CPU load.

So it looks like ctlSQLBox instances are affected, although ctlSQLBox 
itself seems quite harmless (looking at the code), except for maybe 
ctlSQLBox::OnPositionStc() about which I've just no idea. And since 
ctlSQLBox is a direct descendant of a wx class, it looks like maybe it 
is actually wxwin to blame.
So my theory is:
-- either ctlSQLBox::OnPositionStc() itself,
-- or the underlying wx class (classes).

I haven't arranged a development environment yet, but am going to.

Thank you,
Nikolai

---

From: Nikolai Zhubr <[email-masked]>
Date: Sat, 03 Oct 2015 21:56:59 +0300

Hi all,

Ok, I've verified that ctlSQLBox::OnPositionStc() is broken.

If I comment the code for highlighting in it (that is, almost all of its 
body), the problem goes away.

I've found quite some problems observed with it and attempted to fix 
previously, like e.g. the one from 2011:
---------------
+   // Ensure we don't recurse through any paint handlers
+   Freeze();
     UpdateLineNumber();
+   Thaw();

     // Clear all highlighting
---------------
That was most probably only a partial fix, and it was subsequently reverted.

I'd like to also note that 'OnPositionStc' name is pretty much 
misleading. If it was named 'OnPaintedStc' it would be more consistent 
and more hinting where the problem is. I'm aware that renaming alone 
don't usually fix bugs, but it might help humans better catch ones.

Ok, anyway, I've got no real fix for now, but I'm going on.

Thank you,
Nikolai

---

From: Nikolai Zhubr <[email-masked]>
Date: Sun, 04 Oct 2015 01:12:42 +0300

Hi all,

ok, it is broken since this commit:

http://git.postgresql.org/gitweb/?p=pgadmin3.git;a=patch;h=b0ecbbca7f77c0f07cff67bba3d2bca28954a1e2

-   EVT_STC_UPDATEUI(-1,  ctlSQLBox::OnPositionStc)
+   EVT_STC_PAINTED(-1,  ctlSQLBox::OnPositionStc)

It appears that in previous version of scintilla, there existed a 
SCN_POSCHANGED event, which then got obsolete and replaced by 
SCN_UPDATEUI, and probably that is why the name 'OnPositionStc' was 
chosen for the handler. So linking OnPositionStc() to STC_UPDATEUI was 
correct and in accordance with scintilla's manual.
However the change from EVT_STC_UPDATEUI to EVT_STC_PAINTED was 
incorrect. These events are not quite equivalent. EVT_STC_PAINTED has a 
somewhat different purpose.

I've checked, reverting back to EVT_STC_UPDATEUI indeed fixed CPU load 
and lockup issues on windows. See also
http://www.scintilla.org/ScintillaDoc.html#SCN_PAINTED
http://www.scintilla.org/ScintillaDoc.html#SCN_UPDATEUI
http://www.scintilla.org/ScintillaHistory.html
http://web.mit.edu/jhawk/mnt/spo/sandbox/punya/anjuta-1.2.3/doc/ScintillaDoc.html 
  (-- this is some older document with a more detailed explanation of 
SCN_UPDATEUI)

Some relevant excerptions:

"SCN_PAINTED: is to update some _other_ widgets based on a change."

"SCN_POSCHANGED: notification is deprecated as it was causing confusion. 
Use SCN_UPDATEUI instead."

"SCN_UPDATEUI, SCN_CHECKBRACE:
Either the text or styling [...] common use is to check whether the 
caret is next to a brace and set highlights on this brace and its 
corresponding matching brace."

So for me this all sounds clear sufficiently. I'll stick with 
EVT_STC_UPDATEUI and I can rebuild it myself if I have to. Whether fix 
git respectively or not is up to maintainers now.

Thank you,
Nikolai

---

From: Nikolai Zhubr <[email-masked]>
Date: Sun, 04 Oct 2015 01:36:45 +0300

Hi all,

Thank you,
Nikolai

---

From: "J.F. Oster" <[email-masked]>
Date: Sun, 4 Oct 2015 07:32:10 +0300

Hello Nikolai,

Appreciate your research!
I suggest you to send a *.patch (git diff) attachmeht to the list
for the team members and others interested to test it in their
environments.

Sunday, October 4, 2015, 1:36:45 AM, you wrote:

NZ> Hi all,

NZ> ok, it is broken since this commit:

NZ> http://git.postgresql.org/gitweb/?p=pgadmin3.git;a=patch;h=b0ecbbca7f77c0f07cff67bba3d2bca28954a1e2

NZ> -   EVT_STC_UPDATEUI(-1,  ctlSQLBox::OnPositionStc)
NZ> +   EVT_STC_PAINTED(-1,  ctlSQLBox::OnPositionStc)

NZ> It appears that in previous version of scintilla, there existed a 
NZ> SCN_POSCHANGED event, which then got obsolete and replaced by 
NZ> SCN_UPDATEUI, and probably that is why the name 'OnPositionStc' was 
NZ> chosen for the handler. So linking OnPositionStc() to STC_UPDATEUI was
NZ> correct and in accordance with scintilla's manual.
NZ> However the change from EVT_STC_UPDATEUI to EVT_STC_PAINTED was 
NZ> incorrect. These events are not quite equivalent. EVT_STC_PAINTED has a
NZ> somewhat different purpose.

NZ> I've checked, reverting back to EVT_STC_UPDATEUI indeed fixed CPU load
NZ> and lockup issues on windows. See also
NZ> http://www.scintilla.org/ScintillaDoc.html#SCN_PAINTED
NZ> http://www.scintilla.org/ScintillaDoc.html#SCN_UPDATEUI
NZ> http://www.scintilla.org/ScintillaHistory.html
NZ> http://web.mit.edu/jhawk/mnt/spo/sandbox/punya/anjuta-1.2.3/doc/ScintillaDoc.html
NZ>   (-- this is some older document with a more detailed explanation of 
NZ> SCN_UPDATEUI)

NZ> Some relevant excerptions:

NZ> "SCN_PAINTED: is to update some _other_ widgets based on a change."

NZ> "SCN_POSCHANGED: notification is deprecated as it was causing confusion.
NZ> Use SCN_UPDATEUI instead."

NZ> "SCN_UPDATEUI, SCN_CHECKBRACE:
NZ> Either the text or styling [...] common use is to check whether the 
NZ> caret is next to a brace and set highlights on this brace and its 
NZ> corresponding matching brace."

NZ> So for me this all sounds clear sufficiently. I'll stick with 
NZ> EVT_STC_UPDATEUI and I can rebuild it myself if I have to. Whether fix
NZ> git respectively or not is up to maintainers now.

NZ> Thank you,
NZ> Nikolai

>> previously, like e.g. the one from 2011:
....

-- 
Best regards,
 J.F.

---

From: Nikolai Zhubr <[email-masked]>
Date: Sun, 04 Oct 2015 16:52:46 +0300

Hi all,
> for the team members and others interested to test it in their

Well, technically I can, but it will be ugly, see below.

Problem is, the change of STC_UPDATEUI to STC_PAINTED in pgadmin was not 
intended to break pgadmin on Windows, but rather to fix it on Mac. This 
change was obviously incorrect, but simply reverting it would probably 
cause regressions for Mac users. As I personally don't use Mac, I can 
not develop and test a proper fix for it. Therefore, all what I can 
suggest myself is this:

--- ctlSQLBox.cpp.orig	Fri Sep 25 13:20:24 2015
+++ ctlSQLBox.cpp	Sun Oct 04 15:47:17 2015
@@ -40,7 +40,11 @@
  	EVT_MENU(MNU_FIND, ctlSQLBox::OnSearchReplace)
  	EVT_MENU(MNU_AUTOCOMPLETE, ctlSQLBox::OnAutoComplete)
  	EVT_KILL_FOCUS(ctlSQLBox::OnKillFocus)
+#ifdef __WXMAC__
  	EVT_STC_PAINTED(-1,  ctlSQLBox::OnPositionStc)
+#else
+	EVT_STC_UPDATEUI(-1,  ctlSQLBox::OnPositionStc)
+#endif
  	EVT_STC_MARGINCLICK(-1, ctlSQLBox::OnMarginClick)
  	EVT_END_PROCESS(-1,  ctlSQLBox::OnEndProcess)
  END_EVENT_TABLE()

Thank you,
Nikolai

---

From: Dave Page <[email-masked]>
Date: Mon, 5 Oct 2015 15:30:09 +0100

Thanks - patch applied.

On Sun, Oct 4, 2015 at 2:52 PM, Nikolai Zhubr <[email-masked]> wrote:
> change was obviously incorrect, but simply reverting it would probably cause
> regressions for Mac users. As I personally don't use Mac, I can not develop
> and test a proper fix for it. Therefore, all what I can suggest myself is
> this:
>
>> NZ>
>> NZ>

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

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