risoflora / brookframework

Microframework which helps to develop web Pascal applications.
https://risoflora.github.io/brookframework
GNU Lesser General Public License v2.1
239 stars 54 forks source link

In Threading mode URL Router "Freezes" on 404 or "/" #47

Closed gustavogalvan closed 1 year ago

gustavogalvan commented 1 year ago

Maybe I found the problem in BrookURLRouter.pas, in procedure TBrookURLRouter.Route. I put two FLocker.Unlock, one in line 102 after R.HandleRequest(..., and the other at the end, before DoNotFound(...

This is the code fragment:

  if B then
  begin
    DoRoute(ASender, APath, ARequest, AResponse);
    Exit;
  end;
  if APath = '/' then
  begin
    R := FRoutes.FindDefault;
    if Assigned(R) then
    begin
      R.HandleRequest(ASender, R, ARequest, AResponse);
      FLocker.Unlock; {bugfix?}
      Exit;
    end;
  end;
  DoNotFound(ASender, APath, ARequest, AResponse);
  FLocker.Unlock; {bugfix?}
end;

I hope this help. Thank for your great job !

silvioprog commented 1 year ago

Hey @gustavogalvan, thanks a lot for reporting. So, do you have some steps for how to reproduce the problem?

gustavogalvan commented 1 year ago

Yes! Use the provided example in Examples/Console/FPC/urlrouter.lpr and add Threaded:=true; in main section:

begin
  with TServer.Create(nil) do
  try
    Port:=8080;
    Threaded:=true;
    Open;
    if not Active then
      Exit;
    WriteLn('Server running at http://localhost:', Port);
    ReadLn;
  finally
    Free;
  end;
end.

Build and run, and test if work fine in declared routes (like http://127.0.0.1:8080/home). Then try access to / (http://127.0.0.1:8080/) several times and you can see the lock. The same if you try access to a non existent path, like http://127.0.0.1:8080/non-existing-path

This happens in threaded mode and thread-pool.

gustavogalvan commented 1 year ago

Hi, I forgot mention I work in linux.

gustavogalvan commented 1 year ago

testing again, with curl :

gustavo@MOV:~$ curl -w '\n' http://127.0.0.1:8888/non-existing-page Page not found gustavo@MOV:~$ curl -w '\n' http://127.0.0.1:8888/non-existing-page ^C gustavo@MOV:~$ curl -w '\n' http://127.0.0.1:8888/non-existing-page ^C gustavo@MOV:~$ curl -w '\n' http://127.0.0.1:8888/non-existing-page ^C gustavo@MOV:~$

At the first request is ok, then remains locked.

gustavogalvan commented 1 year ago

I followed your code to find the unlocks for that situations and not found. Just two unlocks image

silvioprog commented 1 year ago

Hey @gustavogalvan, could you provide a Pull Request? If so, we can approve it and merge into master.

gustavogalvan commented 1 year ago

Hello Silvio, I just made the pull request. I am sorry for the delay.

El mié, 12 abr 2023 a las 14:12, Silvio Clécio @.***>) escribió:

Hey @gustavogalvan https://github.com/gustavogalvan, could you provide a Pull Request? If so, we can approve it and merge into master.

— Reply to this email directly, view it on GitHub https://github.com/risoflora/brookframework/issues/47#issuecomment-1505632362, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADCBR4INREQ6GVPCJH6JXPDXA3PAXANCNFSM6AAAAAAVVFUPCM . You are receiving this because you were mentioned.Message ID: @.***>

silvioprog commented 1 year ago

Merged. Thanks a lot for the fix! 🙂