tondrej / chakracore-delphi

Delphi and Free Pascal bindings and classes for Microsoft's ChakraCore library
https://tondrej.blogspot.com/search/label/chakracore
MIT License
137 stars 34 forks source link

Call to PostTimedTask fails on Windows (x86_64) in HostSample #20

Closed PascalBergeron1993 closed 1 year ago

PascalBergeron1993 commented 1 year ago

Say we run the following script in the HostSample project:

setTimeout(() => { console.log("(timeout 1000) Current time: ", new Date()) });
setTimeout(() => { console.log("(timeout 1000) Current time: ", new Date()) }, 1000);
setTimeout(c => { c.log("(timeout 1000) Current time: ", new Date()) }, 1000, console);

Such a script will run fine on Windows (i386-win32). However, it will fail on Windows (x86_64) with the following error:

[EAccessViolation] Access violation at address 00007FFBE7308B30 in module 'chakracore.dll'. Read of address 0000000000000010

This issue was observed on Delphi 10.1 Berlin. It seems like this issue happens in the PostTimedTask method because the FuncArgs variable is declared as an array[0..0] of JsValueRef instead of an array of JsValueRef. I was able to fix the issue by changing the PostTimedTask method like so:

function PostTimedTask(Args: PJsValueRefArray; ArgCount: Word; CallbackState: Pointer; RepeatCount: Integer): JsValueRef;
var
  DataModule: TDataModuleMain absolute CallbackState;
  AMessage: TTaskMessage;
  Delay: Cardinal;
  FuncArgs: array of JsValueRef;
  I: Integer;
begin
  Result := JsUndefinedValue;

  if ArgCount < 2 then // thisarg, function to call, optional: delay, function args
    raise Exception.Create('Invalid arguments');

  if ArgCount >= 3 then
    Delay := JsNumberToInt(Args^[2])
  else
    Delay := 0;

  if ArgCount >= 4 then
  begin
    SetLength(FuncArgs, ArgCount - 3);
    for I := 0 to ArgCount - 4 do
      FuncArgs[I] := Args^[I + 3];
  end;

  AMessage := TTaskMessage.Create(DataModule.Context, Args^[1], Args^[0], FuncArgs, Delay, RepeatCount);
  try
    DataModule.Context.PostMessage(AMessage);
  except
    AMessage.Free;
    raise;
  end;
end;

I see that the PostTimedTask method may need to be fixed in the other sample projects.

My fix would also need to be tested on the FreePascal compiler and on the Linux and Darwin platforms.

Let me know your thoughts and if you are open to me making further tests and creating a PR.

tondrej commented 1 year ago

Thank you for taking the time! This is clearly a bug in both HostSample and NodeSample. WasmSample already seems to be fixed. I would simply fix them as in WasmSample, for consistency. I appreciate your help.

tondrej commented 1 year ago

Actually I prefer your fix, it's clearer. 👍 Thanks!

PascalBergeron1993 commented 1 year ago

If I were to test the code further and create a PR, would you accept it and merge it with the master branch?

tondrej commented 1 year ago

Yes.