Converting D6 program to RDS2007 causes crash when closing a subform

[John B]

Hello,

After I recompile my original D6 program in RDS2007, it crashes when closing
a sub form. However, the original executable works fine. I think this may
be related to either the new memory manager in RDS2007. In other words, the
MM may be doing something that has exposed a bug in my code not seen before.
Please help!!!
------
My program is based on an SDI template, but is designed similar to the D6
IDE, with the main form displaying just the menus and toolbars, which then
act upon whatever subform is active. To synch the ActionList on the active
subform with the one on the main form, I assign a custom event handler to
Screen.OnActiveFormChange in the OnCreate event handler of the main form:

procedure T_SDIMain.FormCreate(Sender: TObject);
begin
Screen.OnActiveFormChange := MyActiveFormChanged;
. . .
end;

In the MyActiveFormChanged procedure, I update a private variable of the
Main form to point to the active subform, then use this to call actions of
the appropriate subform based on the user clicking buttons/menus on the main
form:

Type
T_SDIMain = class(TForm)
Private
FActiveViewer : TmcBaseForm;
. . .
end;

procedure T_SDIMain.MyActiveFormChanged (Sender: TObject);
var i: integer;
begin //This code is critical to maintaining a proper link to the apps
baseforms
if (Screen.ActiveForm = Self) then
begin
if not Assigned(FActiveViewer) then
begin //for case when last ActiveViewer is closed
For i := 0 to Screen.FormCount-1 {downto 0} do
if (Screen.Forms[i] is TmcBaseForm) and
((Screen.Forms[i] as TmcBaseForm).Counted) then
begin
Screen.Forms[i].SetFocus; // changes active form to next
mcbaseform
break;
end;
end;
end else
if Screen.ActiveForm is TmcBaseForm then
begin
FActiveViewer := Screen.ActiveForm as TmcBaseForm;
end else
begin
FActiveViewer := nil;
end;
end;

I then use the Update method of the main form's TActionList to synch to
status of the Main form actions to those on the active subform:

procedure T_SDIMain.ActionList1Update(Action: TBasicAction; var Handled:
Boolean);
begin
if Assigned(FActiveViewer) and
(FActiveViewer is TmcBaseForm) then
begin
FActiveViewer.alBaseFormUpdate (Action, Handled); //Calls the update
method of the subform's TActionList

//Set Main actions to enable/disable toolbuttons and menus
actSavePic.Enabled := (FActiveViewer as
TmcBaseForm).actSavePic.Enabled;
actSaveCAD.Enabled := (FActiveViewer as
TmcBaseForm).actSaveCAD.Enabled;
actPrint.Enabled := (FActiveViewer as
TmcBaseForm).actPrint.Enabled;
actPrintPreview.Enabled := (FActiveViewer as
TmcBaseForm).actPrintPreview.Enabled;
end;

My application works fine until I attempt to close a form, after which I get
an Access violation on:
actSavePic.Enabled := (FActiveViewer as
TmcBaseForm).actSavePic.Enabled;

This occurs whether I comment out the line above it or not, nor can I use
the debugger to step through the code, since this involves the Update
method.

Any ideas would be greatly appreciated!!!

John

[Peter Below (TeamB)]

Post by John B
This occurs whether I comment out the line above it or not, nor can I
use the debugger to step through the code, since this involves the
Update method.
Any ideas would be greatly appreciated!!!

This looks like a sequence problem. Obviously the main forms
actionlist1update fires after the child form has been destroyed but
before the screen.OnActiveformChange event has fired and set the
FActiveViewer to nil or to the next active child form.

I would change the strategy you use to track the status of the child
forms. Instead of relying on the Screen.OnActiveformchange event have
the child forms send custom messages to the main form
(Application.Mainform.Perform....) in their OnShow and OnClose events
and use these messages to track the active child. Pass the childs Self
reference as message parameter so you can check on the main form
whether the child announcing its imminent demise is in fact the one
currently stored in FActiveViewer.

There is also one other change you may make: instead of syncing action
on the main form with the child form actions you could actually tie the
appropriate menu items directly to the child form actions when the
active child changes. This way you would save quite a bit of code and
the child form is better placed to update its actions as needed anyway.

If you have a base form class on which all the child forms are based
via visual form inheritance this will be easy to implement through a
change to the base form class. If you don't have a base class you would
have to add this to every child form class you use, or byte the bullet
<g> and redesign the form hierarchy to introduce a base class. This is
a one-time effort: add a new empty form to your project and name it
appropriately. The open the child form s one after the other, add the
base form unit to the interface Uses clause, change the parent class
for the form from TForm to the base form class, switch to the designer,
select "view as text" from the popup menu, change the first "object" of
the dfm file to "inherited", switch back to the form view, save.
Repeat, rinse, dry etc. for the other form classes.

[John B]

Peter, This is great stuff. It's amazing what you can do in Delphi if you
know what you're doing! Wish I did. :-)

Post by Peter Below (TeamB)
I would change the strategy you use to track the status of the child
forms. Instead of relying on the Screen.OnActiveformchange event have
the child forms send custom messages to the main form
(Application.Mainform.Perform....) in their OnShow and OnClose events
and use these messages to track the active child. Pass the childs Self
reference as message parameter so you can check on the main form
whether the child announcing its imminent demise is in fact the one
currently stored in FActiveViewer.

1) Since I use a base form with inheritance for the subforms (not the main
form), would it be better to override the DoShow and DoClose methods in this
class, or to use the OnShow and OnClose events? If used in the ancestor
class, can they be used later in child classes?

2) To make sure I understand how to use Perform, I plan to declare a
constant offset from WM_USER, like:
Const WM_Forms_Changed = WM_USER +555;

Then call it using:
Application.Mainform.Perform (WM_Forms_Changed, Longint(self), MyState)

where MyState is 0 if closing, and 1 if showing. Then in the Main Form, I
need to create a procedure to catch the message, and set my FActiveViewer
variable accordingly. I'll declare it as:

procedure ActiveFormChanged (var Message: TMessage); message
WM_Forms_Changed;

And implement is as:

procedure T_SDIMain.ActiveFormChanged (var Message: TMessage);
begin
if Message.LParam = 0 then {closing}
FActiveViewer := nil
else
FActiveViewer := TMCBaseForm(Message.WParam)
end;
end;

Am I missing anything?

Post by Peter Below (TeamB)
There is also one other change you may make: instead of syncing action
on the main form with the child form actions you could actually tie the
appropriate menu items directly to the child form actions when the
active child changes. This way you would save quite a bit of code and
the child form is better placed to update its actions as needed anyway.

3) This would be great...but how do you tie menu items and speedbuttons in
the main form to actions in the child form. Do I simply include the main
form unit in the uses clause of the base sub form, then link them all
together in the subforms OnShow event or DoShow method? And then set them
back to the main form's actions in subform's OnClose or DoClose?

[Pieter Zijlstra]

It's amazing what you can do in Delphi if you know what you're doing!

It's amazing what some people do, *not* knowing what they are doing ;-)

Wish I did. :-)

Don't we all <sigh> ;-)


PS there are some ";-)" in it, I'm just kidding :)

[John B]

Hey, if we can't laugh at ourselves, we're probably no fun anyway.

John

Post by Pieter Zijlstra

It's amazing what you can do in Delphi if you know what you're doing!

It's amazing what some people do, *not* knowing what they are doing ;-)

Wish I did. :-)

Don't we all <sigh> ;-)
PS there are some ";-)" in it, I'm just kidding :)
--
Pieter

[John B]

All,

Post by John B
procedure T_SDIMain.ActiveFormChanged (var Message: TMessage);
begin
if Message.LParam = 0 then {closing}
FActiveViewer := nil
else
FActiveViewer := TMCBaseForm(Message.WParam)
end;
end;

procedure T_SDIMain.ActiveFormChanged(var Message: TMessage);
var i: integer;
begin
Case Message.LParam of
1: FActiveViewer := TmcBaseForm(Message.WParam);
else
FActiveViewer := nil; //Ref variable in main form
For i := 0 to Screen.FormCount-1 do //Find last active subform
if (Screen.Forms[i] is TmcBaseForm) and //scans only child forms
of ancestor
(LongInt(Screen.Forms[i]) <> Message.WParam) then //ensures
closing form is not selected
begin
FActiveViewer := Screen.Forms[i] as TmcBaseForm; //Sets
reference...Must be done before SetFocus
Screen.Forms[i].SetFocus; // changes active form, as well as
order of Screen.Forms array
break;
end;
End;
end;


Works like a champ now...Thanks, Peter!

John

[Peter Below (TeamB)]

Post by John B
1) Since I use a base form with inheritance for the subforms (not the
main form), would it be better to override the DoShow and DoClose
methods in this class, or to use the OnShow and OnClose events?

Yes, definitely. Using the events would also work but you would have to
make sure that any handlers of the same events on the child forms
contain an "inherited" call to fire the ancestor form event handler.
But overriding the methods that fire the events is always prefered when
writing component classes.

Post by John B
If used in the ancestor class, can they be used later in child

classes?

Yes, no problem. Just remember to call the inherited method or event
handler as well.

Post by John B
2) To make sure I understand how to use Perform, I plan to declare a
constant offset from WM_USER, like: Const WM_Forms_Changed =
WM_USER +555;
Application.Mainform.Perform (WM_Forms_Changed, Longint(self), MyState)
where MyState is 0 if closing, and 1 if showing. Then in the Main
Form, I need to create a procedure to catch the message, and set my
procedure ActiveFormChanged (var Message: TMessage); message
WM_Forms_Changed;
procedure T_SDIMain.ActiveFormChanged (var Message: TMessage);
begin
if Message.LParam = 0 then {closing}
FActiveViewer := nil
else
FActiveViewer := TMCBaseForm(Message.WParam)
end;
end;
Am I missing anything?

Nope, sounds like a plan <g>.

Post by John B

Post by Peter Below (TeamB)
There is also one other change you may make: instead of syncing
action on the main form with the child form actions you could
actually tie the appropriate menu items directly to the child form
actions when the active child changes. This way you would save
quite a bit of code and the child form is better placed to update
its actions as needed anyway.

3) This would be great...but how do you tie menu items and
speedbuttons in the main form to actions in the child form. Do I
simply include the main form unit in the uses clause of the base sub
form, then link them all together in the subforms OnShow event or
DoShow method? And then set them back to the main form's actions in
subform's OnClose or DoClose?

I try to prevent two-way dependencies between units, so adding the main
form unit to a child form unit is a no-no in my opinion. The way I do
this is to implement a mechanism that allows the main form to *ask* the
child form for an action to use for a particular purpose. For this you
need a way to identify the action according to purpose. I do this by
declaring an enumerated type with one element for each action the child
forms will need to support, let's call it TChildActions. The child form
base class then just gets a virtual method

function GetAction( aActionType: TChildActions): TAction; virtual;

If the actionlist that will hold the childforms actions is already
present on the base class *and* if you use the convention to store the
corresponding value of the enumeration into an actions Tag property you
can even implement this function in the base class: just iterate over
the actionlist and look for an action with the correct Tag. Otherwise
the child forms need to override this method.

Tying the actions to the mainform controls would be the main forms task
(a form should not access components on another form directly, that
breaks encapsulation). In your ActiveFormChanged method you have the
proper trigger for doing this task.

[John B]

Peter,

I have implemented your message mechanism for keeping track of the active
subform. My problem now is that when I try to minimize on all the subforms,
one form always seem to remain open. Here's the situation:

To track when a form is minimized or restored, I moved the Perform calls to
overridden Activate, DoClose and Resizing methods of the base class of the
sub forms:

procedure TmcBaseForm.Activate;
begin
inherited;
Application.MainForm.Perform(UM_FORMS_STATUSCHANGE, longint(Self), 1);
end;

procedure TmcBaseForm.Resizing(State: TWindowState);
begin
inherited;
if State = wsMinimized then
Application.MainForm.Perform(UM_FORMS_STATUSCHANGE, longint(Self), 0);
end;

This works fine for manual minimizing. When trying to programmatically
minimize all using code below it does not work. It does work, however, if I
comment out the Resizing calls, except that now the main focus gets messed
up.

procedure T_SDIMain.actMinimizeAllExecute(Sender: TObject);
var i: integer;
begin
For i := 0 to Screen.FormCount-1 do
if (Screen.Forms[i] is TmcBaseForm) and
(Screen.Forms[i].WindowState <> wsMinimized) then
begin
Screen.Forms[i].WindowState := wsMinimized;
end;
end;

Any thoughts on what I am doing wrong?

[Peter Below (TeamB)]

Post by John B
Peter,
I have implemented your message mechanism for keeping track of the
active subform. My problem now is that when I try to minimize on all
the subforms, one form always seem to remain open. Here's the
To track when a form is minimized or restored, I moved the Perform
calls to overridden Activate, DoClose and Resizing methods of the
procedure TmcBaseForm.Activate;
begin
inherited;
Application.MainForm.Perform(UM_FORMS_STATUSCHANGE, longint(Self),
1); end;
procedure TmcBaseForm.Resizing(State: TWindowState);
begin
inherited;
if State = wsMinimized then
Application.MainForm.Perform(UM_FORMS_STATUSCHANGE, longint(Self),
0); end;
This works fine for manual minimizing. When trying to
programmatically minimize all using code below it does not work.

In what manner does it not work? I cannot see your screen from here,
you know.

Post by John B
It
does work, however, if I comment out the Resizing calls, except that
now the main focus gets messed up.
procedure T_SDIMain.actMinimizeAllExecute(Sender: TObject);
var i: integer;
begin
For i := 0 to Screen.FormCount-1 do
if (Screen.Forms[i] is TmcBaseForm) and
(Screen.Forms[i].WindowState <> wsMinimized) then
begin
Screen.Forms[i].WindowState := wsMinimized;
end;
end;
Any thoughts on what I am doing wrong?

Well, after the code above has run it's cause you *know* that all child
forms should be minimized and can set the main forms relevant variables
accordingly, no? No need to rely on the notifcations from the child
forms in this case.

Anyway, you can minimize the child forms in a manner that is closer to
how the user does it. Replace the

Screen.Forms[i].WindowState := wsMinimized;

with

Screen.Forms[i].Perform(WM_SYSCOMMAND, SC_MINIMIZE, 0);

[John B]

Post by Peter Below (TeamB)
In what manner does it not work? I cannot see your screen from here,
you know.

Sorry about that...What is happening is that one of the forms gets skipped
when I iterate through the Screen.Forms array to minimize them all.

I think I figured out what the problem is:

I send a Notification to the main form when minimizing a subform. My
StatusChange code then activates the next form in the Screen.Forms stack to
synch all the menus and speedbuttons. It appears that when I call
Screen.Forms[i].SetFocus to do this, it changes the order of the
Screen.Forms array, so my MinimizeAll loop becomes invalid.

I have two solutions:
1) Set a flag in the MinimizeAll method, then check for that in the
StatusChange method to avoid resetting focus.
2) Create a temporary list from the Screen.Forms array, then iterate through
that.

What would you recommend? Is there an Application.Forms array or something
other than Screen.Forms that I could iterate through that doesn't change
order?

Thanks,

John

[Peter Below (TeamB)]

It appears that when I call Screen.Forms[i].SetFocus to do this, it
changes the order of the Screen.Forms array, so my MinimizeAll loop
becomes invalid.

Yes, I think I dimly remember that the Screen.Forms array gets
reordered if you bring a form to front, since it is the means the VCL
uses to establish a relative Z order between your forms.

1) Set a flag in the MinimizeAll method, then check for that in the
StatusChange method to avoid resetting focus. 2) Create a temporary
list from the Screen.Forms array, then iterate through that.

I would use #2.

What would you recommend? Is there an Application.Forms array or
something other than Screen.Forms that I could iterate through that
doesn't change order?

Application.Components would serve if all forms are owned by the
Application object.

You could also avoid the problem alltogether: if you use

PostMessage (Screen.Forms[i].Handle, WM_SYSCOMMAND, SC_MINIMIZE, 0);

in your loop the actual closing of forms would not start until you have
exited the loop...

[John B]

Post by Peter Below (TeamB)
You could also avoid the problem alltogether: if you use
PostMessage (Screen.Forms[i].Handle, WM_SYSCOMMAND, SC_MINIMIZE, 0);
in your loop the actual closing of forms would not start until you have
exited the loop...

Now you're talking...Boy, I really need to learn more about these messages
and how to use all the functions. :-) They sure seem to be the solution
for a lot of perplexing problems.

I was finally able to get everything working properly after adding a check
in the main form's Activate event. My program now properly opens multiple
subforms, closes them, and minimizes/maximizes/restores them, both
individually and all at once through menu commands, to include properly
synching the menus and toolbars for the active subform, if it exists.

Anyway, Thank you so much...I couldn't have done it without your patient and
expert help. YOU ROCK!

John

[Peter Below (TeamB)]

Post by John B
I was finally able to get everything working properly after adding a
check in the main form's Activate event. My program now properly
opens multiple subforms, closes them, and
minimizes/maximizes/restores them, both individually and all at once
through menu commands, to include properly synching the menus and
toolbars for the active subform, if it exists.

Concrats! You are now entitled to an extra donut <g>.

[Marc Rohloff [TeamB]]

Post by John B
2) To make sure I understand how to use Perform, I plan to declare a
Const WM_Forms_Changed = WM_USER +555;

It is recommended to base your constants on WM_APP and not WM_USER. It
is also reasonably common practice to use UM instead of WM for user
defined messages.

Post by John B
Application.Mainform.Perform (WM_Forms_Changed, Longint(self), MyState)

Since you are using a common base form I would just do something like:
(Application.MainForm as TBaseClass).ActiveFormChanged(self,
MyState)

The only advantage to a message is if you want to delay processing and
unlink it from the current code, but in this case you need to use
PostMessage(Application.MainForm.Handle, WM_Forms_Changed,
Longint(self), MyState);

[John B]

Thanks...I'll make the changes.

Post by Marc Rohloff [TeamB]

Post by John B
2) To make sure I understand how to use Perform, I plan to declare a
Const WM_Forms_Changed = WM_USER +555;

It is recommended to base your constants on WM_APP and not WM_USER. It
is also reasonably common practice to use UM instead of WM for user
defined messages.

Post by John B
Application.Mainform.Perform (WM_Forms_Changed, Longint(self), MyState)

(Application.MainForm as TBaseClass).ActiveFormChanged(self,
MyState)
The only advantage to a message is if you want to delay processing and
unlink it from the current code, but in this case you need to use
PostMessage(Application.MainForm.Handle, WM_Forms_Changed,
Longint(self), MyState);
--
Marc Rohloff [TeamB]
marc -at- marc rohloff -dot- com

[Marc Rohloff [TeamB]]

Post by John B
My application works fine until I attempt to close a form, after which I get

Can you see the callstack at this point?

Post by John B
procedure T_SDIMain.FormCreate(Sender: TObject);
begin
Screen.OnActiveFormChange := MyActiveFormChanged;
. . .
end;

You should add:

procedure T_SDIMain.FormCreate(Sender: TObject);
begin
Screen.OnActiveFormChange := nil;
. . .
end;

To remove your event handler during shutdown. You could also check
Application.Terminated in your event handler.

procedure T_SDIMain.MyActiveFormChanged (Sender: TObject);
begin
if Application.Terminated then exit;

[John B]

Marc, Thanks for you help.

Post by Marc Rohloff [TeamB]
Can you see the callstack at this point?

I can, but I don't really know what I'm looking at (other than it shows the
order of the methods called before the error.) I presume the top of the
list is the place where the error occured.

Post by Marc Rohloff [TeamB]
procedure T_SDIMain.FormCreate(Sender: TObject);
begin
Screen.OnActiveFormChange := nil;
. . .
end;
To remove your event handler during shutdown.

I presume you mean FormClose, and I will do so, and I will also use your
suggestion for checking Application.Terminated in the interim, in case I
can't get Peter's suggestions to work.

Thanks.

John

[Marc Rohloff [TeamB]]

Post by John B

Post by Marc Rohloff [TeamB]
Can you see the callstack at this point?

I can, but I don't really know what I'm looking at (other than it shows the
order of the methods called before the error.) I presume the top of the
list is the place where the error occured.

It gives you a good idea of how and when you got there. It also helps
to turn on debug dcu's in the compiler options so that you get a
complete callstack.

Post by John B
I presume you mean FormClose, and I will do so, and I will also use your
suggestion for checking Application.Terminated in the interim, in case I
can't get Peter's suggestions to work.

No, I meant FormDestroy, I generally try and balance my code, i.e. if
you do something in the OnCreate then it should be undone in
OnDestroy; if you do it in OnShow then undo it in OnClose.

Also don't forget that a form can be closed and reshown.

[John B]

Thanks...

Post by Marc Rohloff [TeamB]

Post by John B

Post by Marc Rohloff [TeamB]
Can you see the callstack at this point?

I can, but I don't really know what I'm looking at (other than it shows the
order of the methods called before the error.) I presume the top of the
list is the place where the error occured.

It gives you a good idea of how and when you got there. It also helps
to turn on debug dcu's in the compiler options so that you get a
complete callstack.

Post by John B
I presume you mean FormClose, and I will do so, and I will also use your
suggestion for checking Application.Terminated in the interim, in case I
can't get Peter's suggestions to work.

No, I meant FormDestroy, I generally try and balance my code, i.e. if
you do something in the OnCreate then it should be undone in
OnDestroy; if you do it in OnShow then undo it in OnClose.
Also don't forget that a form can be closed and reshown.
--
Marc Rohloff [TeamB]
marc -at- marc rohloff -dot- com