Surprising error in a Delphi program

Question

I am using this unit in a Delphi 2010 application to tell me what Active Directory Groups a user is a member of.

I created a brand-new test vcl forms application, added the unit from that link, and made a little form with an edit box for the username, another edit box to hold the CSV separated list of groups, and a list box to hold the list of groups in a columnar format.

My code looks like this:

procedure TfrmMain.btnShowGroupsClick(Sender: TObject);
var
  ad: TADSI;
  adrec: TADSIUserInfo;
  csvGroups: string;
  slGroups: TStringList;
begin
  //take username from an edit box, tell me what AD groups they are a member of
  ad := TADSI.Create(Self);
  try
    ad.GetUser(edtDomain.Text,edtUser.Text,adrec);
    csvGroups := adrec.Groups;
    edtADGroups.Text := csvGroups;  //ACCESS VIOLATION!!
  finally
    FreeAndNil(ad);
  end;

  {
  //If I UN-comment this code, and make NO OTHER CHANGES, then the
  //aforementioned access violation does NOT occur; there are no errors @ all,
  //and everything works just fine

  slGroups := TStringList.Create;
  try
    slGroups.CommaText := csvGroups;
    listBoxADGroups.Items := slGroups;
  finally
    FreeAndNil(slGroups);
  end;
  //}
end;

If I run this code as-is, I get an access violation when I try to assign the CSV list of group to the edit box.

---------------------------
Debugger Fault Notification
---------------------------
Project C:\Users\my_username.mydomain\bin\ADSITest.exe faulted with message: 'access violation at 0x0048a321: read of address 0x458c0035'. Process Stopped. Use Step or Run to continue.
---------------------------
OK   
---------------------------

However, if I un-comment the block of code involving the TStringList, then it all works great.

Either this is some really weird compiler bug, or I'm missing something obvious. Can someone help me out?

The "adrec" structure is a simple record consisting of a few booleans, strings, and one other record (TPassword).

Answer 1

When you do listBoxADGroups.Items := slGroups; you are replacing listBoxADGroups.Items by a pointer to slGroups, a couple of lines below, you are freeing it. When the program ends your btnShowGroupsClick method, the ListBox tries to use this pointer, but it is nil!, hence, the A.V.

A solution is this:

  slGroups := TStringList.Create;
  try
    slGroups.CommaText := csvGroups;
    listBoxADGroups.Items.AddStrings(slGroups);
  finally
    FreeAndNil(slGroups);
  end;

The AddStrings method copies the contents of slGroups to listBoxAdGroups.Items property (which is a TStrings object too), instead of just replacing a pointer. This way, the Items property of TListBox is intact, only its content was changed.

Answer 2

I see that in your code you also get an AV on the line:

edtADGroups.Text := csvGroups;  //ACCESS VIOLATION!!

In the section involving the TStringList, I bet you get the exception on the line:

listBoxADGroups.Items := slGroups;

I think that Self is invalid somehow, maybe it's been free'd somewhere...

Are you calling that method externally, or are you clicking on the button?

Answer 3

I haven't reproduced the problem or examined the output of the compiler for this code, but I notice that when that block of code is commented out, the csvGroups variable is redundantly used to store and load a value from one location into another.

This makes me wonder if the compiler is perhaps prematurely optimising away some crucial housekeeping around that string variable.

The commented out code contains a further reference to csvGroups, thus extending the life of that variable and potentially defeating the optimisation that the compiler is erroneously performing.

To test this theory I would eliminate the use of that variable entirely from the "as-is" version of the code, i.e. change:

csvGroups := adrec.Groups;
edtADGroups.Text := csvGroups;  //ACCESS VIOLATION!!

to simply:

edtADGroups.Text := adrec.Groups;

Answer 4

This is speculation, but it looks to me like the unit you're using is not UNICODE compatible. If you don't get any group back (did you debug and look at the return?) cvsGroups is probably not terminated properly. When you uncomment the code, writing to slGroups probably overwrites whatever junk you had before (or at least the compiler does something like initializing slGroups once it's clear it will be touched).