Access violation if compiled for Win32/Win64 (scanning big QRCodes doesn't work at all). Fix to be evaluated proposed here

[csm101]

I think that in unit Detector.pas, at the end of TDetector.processFinderPatternInfo there is "FreeAndNil" being called on an object that has not been dinamically allocated (so it can't be freed) and this causes an access violation. The error does not happen for Ios/Android because FreeAndNil on such platforms does not free anything: it just sets the pointer to nil.

The fact that the problem is that you are trying to free a constant object is just a supposition of mine... I didn't really dig into the root cause, but it is sure that this FreeAndNil on Win32/Win64 causes an acceess violation if you try to scan the qrcode I am attaching to this bugreport.. and if you replace the freeand nil with a plain nil assignment the qrcode gets recognised correctly. I don't know it this leads to memory leaks, though... Anyway I noticed that this code has some logical issues after that FreeAndNil (see my comments): This is my "fix":

      transform := TDetector.createTransform(topLeft, topRight, bottomLeft, AlignmentPattern, dimension);
      try
        bits := TDetector.sampleGrid(FImage, transform, dimension);
      finally
        FreeAndNil(transform);
        //    FreeAndNil(AlignmentPattern); << THIS CAUSES AN ACCESS VIOLATION ON WIN32/WIN64!!
        AlignmentPattern := nil; // << MY FIX: JUST SET IT TO NIL
      end;

      if (bits = nil) then
      begin
        Result := nil;
        exit
      end;

      // PERSONAL NOTE: IT IS IMPOSSIBLE TO GET HERE HAVING AlignmentPattern <> nil!!!
      // the finally block above will be executed ALWAYS and it will alwais set AlignmentPattern to NIL
      if (AlignmentPattern = nil) then  
        points := TArray<TResultpoint>.Create(bottomLeft, topLeft, topRight)
      else // the "ELSE" code will NEVER be executed
        points := TArray<TResultpoint>.Create(bottomLeft, topLeft, topRight,AlignmentPattern);

      Result := TDetectorResult.Create(bits, points);

end;

Here is the QR-Code I am trying to recognize that causes the error (I took it directly from the wikipedia page about qrcodes). If I remove that FreeAndNil it gets recognized correctly

[csm101]

Ok, my fix is wrong: I found the root cause of the access violation: the problem is in "function TDetector.findAlignmentInRegion": under Win32/Win64 this function returns a pointer to an object that has already been deallocated.

These are the last lines of TDetector.findAlignmentInRegion;

  Result := alignmentFinder.find;
  FreeAndNil(alignmentFinder); // this freeAndNil, under Windows, destroys also the object being returned!!
end;

This happens because the object being returned is also kept inside the "possibleCenters" member variable of alignmentFinder. If you destroy AligmnetfFinder you destroy also all instances kept in possibleCenters, including the one being returned.

The access violation I reported actually was caused by the fact that the program is trying to free the same object twice.

Suggested solution: modify the above code in order to return a CLONE of the result of AlignmentFinder.Find

[csm101]

I have issued a merge request with a fix for this bug

[Spelt]

Thanks for the feedback!

I will take a look at it and report my findings back. I will also add this barcode to the unit tests. If I have a question I will get back to you.

I'll will merge the changes if successful into 3.0 https://github.com/Spelt/ZXing.Delphi/tree/v_3.0

[Spelt]

The merge request has been applied. I also added another test case for testing the very large qr code.

The changes are added manually to the v3 branch.

The other issues I will add at another time,

Thank you for taking the time to improve on this lib !

Op 11 jul. 2016, om 19:15 heeft Carlo Sirna notifications@github.com het volgende geschreven:

I have issued a merge request with a fix for this bug

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/Spelt/ZXing.Delphi/issues/19#issuecomment-231801006, or mute the thread https://github.com/notifications/unsubscribe/AMEt1XPFSWNwcunZgk5V8im28Ot-dGAfks5qUnowgaJpZM4JJLsm.

[Spelt]

Hi

All issues are committed and pushed to github. Thanks for the help!

Op 11 jul. 2016, om 19:15 heeft Carlo Sirna notifications@github.com het volgende geschreven:

I have issued a merge request with a fix for this bug

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/Spelt/ZXing.Delphi/issues/19#issuecomment-231801006, or mute the thread https://github.com/notifications/unsubscribe/AMEt1XPFSWNwcunZgk5V8im28Ot-dGAfks5qUnowgaJpZM4JJLsm.

[csm101]

Thank you for having ported this library to delphi! P.S: now I am running some tests about memory allocation using AQTime from smartbear... I'll let you know if i find some other problem