PD 3 Beta Finalize/DoneDevice bug! (+ temporary workaround)

[doggo18]

Bleh... bugs DO exist

In TPowerDraw.Destroy, you call Finalize.
Finalize calls DoneDevice which executes the handler and so forth.
Usually this is no problem, but for my program, I have to Finalize Powerdraw already in the OnDestroy handler of my form myself, because I have a few helperclasses which PowerFont's and Imagelists rely on TPowerDraw and as you know those need to know about the DoneDevice event. So I called the proper method in my helperclasses to ensure everything was finalized properly.

Then I got an AV. What went wrong? Your TPowerDraw.Destroy calls Finalize. Finalize in turn, calls DoneDevice. Finalize nor DoneDevice performs any checking whatsoever if the device has already been 'done'. So in the second line of the TPowerDraw.DoneDevice my event handler was called again. Only, this time, all my objects were already freed. After some debugging I found this workaround:

[pascal]
procedure TMForm.FormDestroy(Sender: TObject);
begin
PowerDraw.Finalize; {<-- this line HAS to be here! }

while FSceneList.Count > 0 do
begin
TObject(FSceneList[0]).Free;
FSceneList.Delete(0);
end;

FSceneList.Free;

FResourceKeeper.Free;
FConfigManager.Free;
end;

...

procedure TMForm.PowerDrawDoneDevice(Sender: TObject);
var
i: Integer;
begin
{ The next line guards us against a nasty AV caused by a double call
to DoneDevice without checking if the device was already done }

if not PowerDraw.DeviceActive then
Exit;

{ First notify scenes }

for i := 0 to FSceneList.Count - 1 do
begin
TScene(FSceneList[i]).DoneDevice;
end;

{ Now, notify Resource Keeper }

FResourceKeeper.DoneDevice;
end;
[/pascal]

Sorry about the long explanation, tried to explain as well as I could. Anyway.. I hope this might help some people who get a nasty AV like me

[Edit]After some thinking, I think Finalize needs an extra check on FInitialized and DoneDevice on FDeviceActive. This way, it's supersafe [/Edit]

[doggo18]

Sorry to find so many.. Well glad this is still the beta release

In the TVTDb class, you have an initialized property, with FInit being the 'reader' for the property. Use use the variable maybe 5 times: one time in the Create (FInit := False and as checks in SetFileName, SetOpenMode and other properties.

My point: You have forgotten to set FInit to True after a succesful Initialize;

My temporary bugfix inside VTDbUnit.pas (TVTDb.Initialize, line 275):

[pascal]
// Read-only mode opens existing file
opReadOnly:
Result:= UpdateKeys();
end;

if Result = errNone then { Lines added since they should have }
FInit := True; { been here in the first place.. }
end;[/pascal]

While I am at it... :twisted:

[pascal]
TOpenModes = (opUpdate, opOverwrite, opReadOnly);
[/pascal]

This is the declaration of TOpenModes. For as far I am aware the 'standard' is that the first few letters of an enumerated type's value are the first letters of the words the name of the enumerated type is composed from. (TBorderStyle - bsNone, bsSingle or TWindowState - wsMinimized, wsMaximized). So I think the declaration should be this:

[pascal]
TOpenModes = (omUpdate, omOverwrite, omReadOnly);
[/pascal]

No need to thank me :twisted: :twisted:

[Edit]
BlueCat, it seems the BOLD and perhaps the other styles too, act weird when placed inside a pascal tag. Wanted to use them for clearification

:idea: They should declare May 31 as a International Bugfinding festival :idea:
[/Edit]

[LP]

To ilustrate what you do wrong with PowerDraw.Finalize:
Create a TEdit control, name it Edit1.
Now, on your OnDestroy event put this line:
Edit1.Free;

same applies to PowerDraw.Finalize, although I must agree that's not the best way it's implemented...

[doggo18]

Not really, in other components (DelphiX being one) I could Finalize just fine. What I understand with Finalize is: undo everything that the initialization did, and put things back in the proper state. Besides, if you code clear and would put FreeAndNil(Edit1)... but I don't have that option with PowerDraw do I?

[LP]

PowerDraw makes sure DoneDevice and Finalize are called when the component is destroyed, i.e. PowerDraw.Destroy, which is called upon PowerDraw.Free() method. Main form's OnDestroy event happends when all form's owned components are released (PowerDraw.Owner = main form), so when you call PowerDraw.Finalize, it no longer exists, so when it accesses its own properties it'd give you Access Violation. In OnDestroy event of your main form you'd free memory and do finalization tasks for anything that was created in run-time or in OnCreate event. Well, at least that's how I understand it, any corrections are welcome

Edit:
It IS possible to do FreeAndNil(PowerDraw), and you're right about Finalize and that's exactly what PowerDraw.Finalize does
If you want to be able to use Finalize on OnDestroy event, you'd create PowerDraw in run-time, in OnCreate event: PowerDraw:= TPowerDraw.Create(nil), and then the following is valid in OnDestroy event:
PowerDraw.Finalize();
PowerDraw.Free();

Edit2:
Indeed the Initialized property of TVTDb was never set to True, just fixed it... still, I think it's good to wait till our new component pack comes out, hopefully it'll be soon

Hope this helps,
Lifepower

[doggo18]

NAh you are wrong here
I just doublechecked, and the VCL first sends the OnDestroy event, and only after that it begins destroying the owned components.
Also, the AV was caused because your Finalize procedure already freed a few objects and interfaces, and didn't check if it could 'free' them again. One way to solve this is by adding the FInitialized and FDeviceActive checks, another would be to set every handle to nil (but would be a more dirty tho). Feel free to try again, but I am not convinced yet; there should be a simple if-check to prevent the AV from occuring.

(Or did you see other VCL controls give an AV because you did something that's not possible? For as far I know they always check if they can do it.)

Another reason I just came up with : Finalize is a public property, so it should be possible to call it whenever I would like to. (My choice: OnDestroy )

Thanks for fixing the VTDb bug, and for this nice conversation heh

[LP]

Hmm, the only reason Access Violation occurs is because PowerDraw memory is freed (that is, it doesn't exist in OnDestroy event). Finalize method frees politely its stuff by "nil" comparison. The event order I've told you about was found practically. You might want to check it out by adding some logging or simply ShowMessage, put it in PowerDraw.Finalize method, and then another one in Destroy method. You'll notice that PowerDraw gets destroyed and finalized BEFORE OnDestroy method occurs (well, that's how I found it out).
Finally, thanks a lot for pointing out some bugs and any suggestions you have made

[doggo18]

I had an easier way of checking.
I went to TCustomForm.Destroy, and I checked the code there.
The DoDestroy gets raised there BEFORE inherited Destroy;. A few Control clicks revealed that TControl.Destroy goes by all owned components, and frees them. Unless Powerdraw is written at a weird way , or somehow all my sourcefiles got tampered, I still think that the OnDestroy event gets called BEFORE the components are freed.

Well another easy way of checking. Query Edit1.Text in your OnDestroy, and show it in a message box. You might not see the messagebox, since the application is being terminated, but if there is no AV the edit control must still have existed

But well.. an AV occuring in -ANY- component, is a bug. If the user (programmer in this case) can cause it without making THackXXX descendants or other dirty tricks, I see no excuses for allowing an AV to happen. I don't have Delphi available right now, but did you set the pointers to nil after freeing the stuff the AV comes with? Otherwise it's not useful to check for nil

I think I'll stop posting in this thread.. I don't want to appear hostile... yet