7

In an attempt to use the threading library in Delphi to calculate tasks in parallel and using TTask.WaitForAny() to get the first calculated result, an exception occationally stopped the execution.

Call stack at the exception:

First chance exception at $752D2F71. Exception class EMonitorLockException with message 'Object lock not owned'. Process Project1.exe (11248)

:752d2f71 KERNELBASE.RaiseException + 0x48
System.TMonitor.CheckOwningThread
System.ErrorAt(25,$408C70)
System.Error(reMonitorNotLocked)
System.TMonitor.CheckOwningThread
System.TMonitor.Exit
System.TMonitor.Exit($2180E40)
System.Threading.TTask.RemoveCompleteEvent(???)
System.Threading.TTask.DoWaitForAny((...),4294967295)
System.Threading.TTask.WaitForAny((...))
Project9.Parallel2
Project9.Project1
:74ff919f KERNEL32.BaseThreadInitThunk + 0xe
:7723b54f ntdll.RtlInitializeExceptionChain + 0x8f
:7723b51a ntdll.RtlInitializeExceptionChain + 0x5a

The call stack leads to the conclusion that the exception is caused by a bug in the threading library, TMonitor and/ or TTask.WaitForAny(). To verify that, the code was cut down to a minimum:

program Project1;

{$APPTYPE CONSOLE}

uses
  System.SysUtils, System.Threading, System.Classes, System.SyncObjs,
  System.StrUtils;
var
  WorkerCount : integer = 1000;

function MyTaskProc: TProc;
begin
  result := procedure
    begin
      // Do something
    end;
end;

procedure Parallel2;
var
  i : Integer;
  Ticks: Cardinal;
  tasks: array of ITask;
  LTask: ITask;
  workProc: TProc;
begin
  workProc := MyTaskProc();
  Ticks := TThread.GetTickCount;
  SetLength(tasks, WorkerCount); // number of parallel tasks to undertake
  for i := 0 to WorkerCount - 1 do // parallel tasks
    tasks[i] := TTask.Run(workProc);
  TTask.WaitForAny(tasks); // wait for the first one to finish
  for LTask in tasks do
    LTask.Cancel; // kill the remaining tasks
  Ticks := TThread.GetTickCount - Ticks;
  WriteLn('Parallel time ' + Ticks.ToString + ' ms');
end;

begin
  try
    repeat
      Parallel2;
      WriteLn('finished');
    until FALSE;
  except
    on E: Exception do
      writeln(E.ClassName, ': ', E.Message);
  end;
  Readln;
end.

Now the error reproduces after a while and the RTL bug is verified.

This was submitted as RSP-10197 TTask.WaitForAny gives exception EMonitorLockException "Object lock not owned" to Embarcadero.


Given the fact that this is currently not possible to solve with the Delphi threading library, the question is:

Is there a workaround to execute a procedure in parallel to get the first acquired solution?

David Heffernan
  • 601,492
  • 42
  • 1,072
  • 1,490
Jimmy Dean
  • 153
  • 2
  • 7
  • can you see the revised code because I can't – Jimmy Dean Mar 12 '15 at 14:35
  • Rewritten what code? I don't see any code. If this is about your previous question then asking a new question like this is not appropriate. – J... Mar 12 '15 at 14:35
  • The system did not include the revised code. It now says I need to wait at least 90 minutes before I can post it. – Jimmy Dean Mar 12 '15 at 14:45
  • 2
    @JimmyDean You should not re-ask the question - you should go back to your old question and *EDIT* it with the new information. – J... Mar 12 '15 at 16:56
  • 1
    You aren't running 1000 tasks in parallel. The scheduler decides how many to run at any one time. – David Heffernan Mar 12 '15 at 18:26
  • 1
    Additionally, this type of question *(ie :"Is my code optimized? Please check it.")* is entirely off-topic. With few exceptions, code can almost always be further optimized and the question is extremely open-ended. In this case, the code is such a disaster that a comprehensive answer would be a very large task indeed. You got a wealth of good advice in your last question and your "revised" code seems to have ignored much or most of it. Start with something smaller and try to learn each piece one at a time - leaping into a complex problem with no plan will never work out well. – J... Mar 12 '15 at 18:46
  • 1
    I still see many infractions in this code that you were advised against in the previous iteration of this question. Namely, scads of global variables, calling functions from multiple threads that are known to not be thread-safe, and atrocious indentation. It's *way* too early for you to be talking about optimizing this code. Optimization is only meaningful for *correct* code, and your code isn't anywhere close to correct yet. – Rob Kennedy Mar 12 '15 at 18:47
  • 1
    @JimmyDean Contrary to your belief that the rules of this site are "short-sighted and petty": The rules have evolved over years with the objective of keeping this site a **high-quality** Q & A reference. The question you have asked is not conducive to the objectives of the site, nor are your attempts to create duplicates that have only minor variation. If you follow the guidelines of the site, I'm sure you'll find it an invaluable resource. And in time, you'll come to understand why some of the seemingly harsh rules are in place. – Disillusioned Mar 12 '15 at 19:33
  • You are not running 1000 tasks at the same time. You are submitting them and scheduler decides how many to run at any one time. You code is beyond saving. As you can see, you've been given that message by many people now. Why ask if you have no intention of listening? – David Heffernan Mar 13 '15 at 14:48
  • 1
    This almost has a sensible question now. Voting to reopen. – J... Mar 14 '15 at 09:59
  • 1
    @J..., I added the error call stack. Points to TMonitor. My home computer has problems reproducing this error, only 1 run in 15 causes it, while my job computer has 2 of 3. – LU RD Mar 14 '15 at 10:51
  • 1
    @LURD I just wrap the outer code in a for loop and get the error every time – David Heffernan Mar 14 '15 at 20:39
  • 2
    @DavidHeffernan, thanks. I cut down code to a minimum reproducable example. I'll submit a QC. Clearly there is a bug either in TMonitor or TTask.WaitForAny. – LU RD Mar 14 '15 at 22:51
  • @LURD or possibly both – David Heffernan Mar 14 '15 at 22:58
  • @DavidHeffernan, see [TTask.WaitForAny gives exception EMonitorLockException "Object lock not owned"](https://quality.embarcadero.com/browse/RSP-10197). – LU RD Mar 14 '15 at 23:09
  • Is this Question still on Hold? Have a stumbled across a bug(s) in Delphi XE7 – Jimmy Dean Mar 14 '15 at 23:51
  • Yes, hopefully enough votes will be cast to reopen this, so I can submit an answer. Yes, you hit a showstopper. Meanwhile look for the [OTL](http://www.omnithreadlibrary.com/) framework, which is widely used and a more proven concept for parallel programming. And don't forget to follow the advice given by David. There are still lots of things to improve. – LU RD Mar 14 '15 at 23:57
  • @LURD I must personally thank you for persevering with the code. By your interest you kept the issue alive. Otherwise this bug(s) in Delphi XE7 would not have been discovered and my question would have remained unanswered as no-one would have bothered to it any meaningful thought. – Jimmy Dean Mar 15 '15 at 00:18
  • Lots of us gave it serious thought. What was needed was simpler code. I asked you to produce simpler code. You declined. @LURD did that, and as invariably happens, the simplification led to clarity. – David Heffernan Mar 15 '15 at 07:11
  • @DavidHeffernan and others who placed this question on hold, could you now unhold it so I can see the answer that LURD would like to submit and perhaps others who may find a solution contsructed in an entirely different way. Thanks – Jimmy Dean Mar 15 '15 at 10:21
  • I don't want to vote to reopen. The fact that LURD has identified what seems to be an RTL bug in his code, inspired by your code, does not make the question, in its current form, a good question. If the question was edited to contain the code from @LURD's bug report that would change it. – David Heffernan Mar 15 '15 at 12:44
  • 1
    @DavidHeffernan I know its another question but how do I remove all my code and enter the bug report of LURD. Then would my question just be object lock not owned error please resolve? or would LURD have to do it. I want to follow your protocols but I am an extreme novice and do require some guidance. Thankyou – Jimmy Dean Mar 15 '15 at 13:01
  • 1
    I would remove everything, post @LURd's code, ask how to workaround the problem. – David Heffernan Mar 15 '15 at 13:36
  • @LURD thank you for providing the code snipet and updating the question. Hopefully now the question will no longer be on hold. – Jimmy Dean Mar 16 '15 at 02:31
  • 1
    The bug report (RSP-10197) is currently reported as fixed in Delphi 10 Seattle. But changing the WorkerCount to a small number (4) still reproduces the bug. So we will have to wait for next version to see if they can get it sorted out. – LU RD Sep 02 '15 at 11:34

1 Answers1

4

Here is an example using TParallel.For to stop the execution when an answer is produced. It uses the TParallel.LoopState to signal other members of the parallel for loop. By using the .Stop signal, all current and pending iterations should stop. Current iterations should check loopState.Stopped.

procedure Parallel3(CS: TCriticalSection);
var
  Ticks: Cardinal;
  i,ix: Integer;  // variables that are only touched once in the Parallel.For loop
begin
  i := 0;
  Ticks := TThread.GetTickCount;
  TParallel.For(1,WorkerCount,
    procedure(index:Integer; loopState: TParallel.TLoopState)
    var
      k,l,m: Integer;
    begin
      // Do something complex
      k := (1000 - index)*1000;
      for l := 0 to Pred(k) do
        m := k div 1000;
      // If criteria to stop fulfilled:
      CS.Enter;
      Try
        if loopState.Stopped then // A solution was already found
          Exit;
        loopState.Stop;  // Signal 
        Inc(i);
        ix := index;
      Finally
        CS.Leave;
      End;
    end
  );
  Ticks := TThread.GetTickCount - Ticks;
  WriteLn('Parallel time ' + Ticks.ToString + ' ticks', ' i :',i,' index:',ix);
end;

The critical section protects the calculated results, here for simplicity i,ix.


Disclaimer, given the state of bugs galore within the System.Threading library, I would recommend another solution using the OTL framework. At least until the library has reached a stable foundation.

LU RD
  • 34,438
  • 5
  • 88
  • 296
  • I am trying to use the OTL framework to reproduce your proposed solution. What is the equivalent to LoopState in the OTL. Would I have to use a combination of Stopped and cancellationToken. Thanks – Jimmy Dean Apr 15 '15 at 04:31
  • cancelToken.Signal to set and cancelToken.IsSignaled to test should do it. Parallel.ForEach(1,WorkerCount) .CancelWith(cancelToken) .Execute(procedure( const index: integer) begin ... end); – LU RD Apr 15 '15 at 06:44