Results 1 to 8 of 8

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

  1. #1

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

    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]

  2. #2

    Another buggy..

    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]

  3. #3

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

    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...

  4. #4

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

    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?

  5. #5

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

    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

  6. #6

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

    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

  7. #7

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

    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

  8. #8

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

    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

Bookmarks

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •