2015-01-03 5 views
7

System.Generics.Collections.TArray.Copy<T> (already reported bug in System.CopyArray के आधार पर) के साथ एक अप्रत्याशित बग की वजह से कभी-कभी थ्रेडिंग लाइब्रेरी का उपयोग करके अपवाद उठाएगा।TSparseArray <T> को कैसे ठीक करें?

अपवाद विधि System.Threading.TSparseArray<T>.Add में उठाया जाता है:

function TSparseArray<T>.Add(const Item: T): Integer; 
var 
    I: Integer; 
    LArray, NewArray: TArray<T>; 
begin 
    ... 
      TArray.Copy<T>(LArray, NewArray, I + 1); // <- Exception here 
    ... 
end; 

खैर कि System.CopyArray में बग के साथ की उम्मीद है। तो जब इसे ठीक करने का प्रयास किया गया तो मेरा पहला विचार केवल सरणी को प्रतिलिपि बनाना था:

// TArray.Copy<T>(LArray, NewArray, I + 1); // <- Exception here 
for LIdx := Low(LArray) to High(LArray) do 
    NewArray[LIdx] := LArray[LIdx]; 

एक आकर्षण की तरह काम करता है। लेकिन उस के बाद मैं सोच रहा था, क्यों सरणी प्रतिलिपि की जरूरत है:

LArray := FArray; // copy array reference from field 
... 
SetLength(NewArray, Length(LArray) * 2); 
TArray.Copy<T>(LArray, NewArray, I + 1); 
NewArray[I + 1] := Item; 
Exit(I + 1); 

तत्वों (स्थानीय चर) को कॉपी कर रहे हैं और यह thats। FArray पर कोई असाइनमेंट नहीं है, इसलिए मुझे को दायरे से बाहर कर दिया जाएगा।

अब मैं तीन बग सुधार विकल्प हैं:

  1. बस की जगह TArray.Copy

    SetLength(NewArray, Length(LArray) * 2); 
    // TArray.Copy<T>(LArray, NewArray, I + 1); // <- Exception here 
    for LIdx := Low(LArray) to High(LArray) do 
        NewArray[LIdx] := LArray[LIdx]; 
    NewArray[I + 1] := Item; 
    Exit(I + 1); 
    
  2. बदलें TArray.Copy और (बाहर बचाने

    SetLength(NewArray, Length(LArray) * 2); 
    // TArray.Copy<T>(LArray, NewArray, I + 1); // <- Exception here 
    for LIdx := Low(LArray) to High(LArray) do 
        NewArray[LIdx] := LArray[LIdx]; 
    NewArray[I + 1] := Item; 
    FArray := NewArray; 
    Exit(I + 1); 
    
  3. टिप्पणी सभी अनावश्यक कोड भागों क्योंकि टी हे सिर्फ समय बर्बाद कर रहे)

    // SetLength(NewArray, Length(LArray) * 2); 
    // TArray.Copy<T>(LArray, NewArray, I + 1); // <- Exception here 
    // NewArray[I + 1] := Item; 
    Exit(I + 1); 
    

मैं अप्रयुक्त कार्यकर्ता धागे या नहीं निष्पादित कार्यों के लिए की तलाश में कार्यों की एक समूह के साथ सभी तीन सुधारों की जाँच की। लेकिन मुझे उनमें से कोई भी नहीं मिला। पुस्तकालय अपेक्षित काम करता है (और अब बिना किसी अपवाद के)।

क्या आप कृपया मुझे बता सकते हैं कि मैं यहां क्या खो रहा हूं?


इस अपवाद के लिए, आप कार्यों की एक गुच्छा चलाने के लिए और TTaskPool अधिक से अधिक TWorkerQueueThreads बना सकते हैं करने के लिए। टास्कमेनर द्वारा थ्रेड की गिनती की जांच करें और TSparseArray<T>.Add विधि में TArray.Copy लाइन पर ब्रेकपॉइंट का उपयोग करें। यहां मुझे यह अपवाद मिलता है जब आवेदन की धागा गिनती 25 धागे से अधिक हो जाती है।

// Hit the button very fast until the debugger stops 
// at TSparseArray<T>.Add method to copy the array 
procedure TForm1.Button1Click(Sender : TObject); 
var 
    LIdx : Integer; 
begin 
    for LIdx := 1 to 20 do 
    TTask.Run( 
     procedure 
     begin 
     Sleep(50); 
     end); 
end; 
+0

यहां तक ​​कि अगर 'TArray.Copy()' नियत ना मुझे लगता है कि होगा मिल जाएगा होगा 'फेयर्रे' का लापता असाइनमेंट/संशोधन एक अलग बग होगा जिसे भी ठीक करने की आवश्यकता है। अन्यथा सरणी में 'आइटम' जोड़ना 'जोड़ें') कैसे है? मैंने अभी तक 'TSparseArray' के स्रोत कोड को नहीं देखा है। –

+0

'TSparseArray ' अजीब है। अलग लॉक ऑब्जेक्ट बनाने की आवश्यकता क्यों महसूस होती है? खुद को लॉक करने में क्या गड़बड़ है? –

उत्तर

1

इससे कोई फर्क नहीं पड़ता कि आइटम TSparseArray<T> पर लिखे गए हैं, क्योंकि केवल तभी आवश्यक है जब एक कार्यकर्ता थ्रेड ने उन्हें दिए गए सभी कार्यों को समाप्त कर दिया हो और अन्य कार्यकर्ता धागा अभी तक समाप्त नहीं हुआ है। इस बिंदु पर निष्क्रिय धागा पूल के अंदर के अन्य हिस्सों की कतारों को देख रहा है और कुछ काम चुरा लेने की कोशिश करता है।

यदि कोई कतार इस सरणी में नहीं आई है तो निष्क्रिय धागे को दिखाई नहीं दे रहा है और इसलिए काम करने वाले लोड को साझा नहीं किया जा सकता है।

कि मैं विकल्प 2

function TSparseArray<T>.Add(const Item: T): Integer; 
... 
SetLength(NewArray, Length(LArray) * 2); 
TArray.Copy<T>(LArray, NewArray, I + 1); // <- No Exception here with XE7U1 
NewArray[I + 1] := Item; 
{$IFDEF USE_BUGFIX} 
FArray := NewArray; 
{$ENDIF} 
Exit(I + 1); 

चयन ठीक करने के लिए लेकिन वह हिस्सा चोरी जोखिम भरा किसी भी ताला लगा

procedure TThreadPool.TQueueWorkerThread.Execute; 

... 

if Signaled then 
begin 
    I := 0; 
    while I < Length(ThreadPool.FQueues.Current) do 
    begin 
    if (ThreadPool.FQueues.Current[I] <> nil) 
     and (ThreadPool.FQueues.Current[I] <> WorkQueue) 
     and ThreadPool.FQueues.Current[I].TrySteal(Item) 
    then 
     Break; 
    Inc(I); 
    end; 
    if I <> Length(ThreadPool.FQueues.Current) then 
    Break; 
    LookedForSteals := True; 
end 

सरणी लंबाई केवल इतना बढ़ रहा है

while I < Length(ThreadPool.FQueues.Current) do 
बिना लागू किया है

और

if I <> Length(ThreadPool.FQueues.Current) then 

पर्याप्त सुरक्षित होना चाहिए।

if Signaled then 
begin 
    I := 0; 
    while I < Length(ThreadPool.FQueues.Current) do 
    begin 
    {$IFDEF USE_BUGFIX} 
    TMonitor.Enter(ThreadPool.FQueues); 
    try 
    {$ENDIF} 
     if (ThreadPool.FQueues.Current[I] <> nil) and (ThreadPool.FQueues.Current[I] <> WorkQueue) and ThreadPool.FQueues.Current[I].TrySteal(Item) then 
     Break; 
    {$IFDEF USE_BUGFIX} 
    finally 
     TMonitor.Exit(ThreadPool.FQueues); 
    end; 
    {$ENDIF} 
    Inc(I); 
    end; 
    if I <> Length(ThreadPool.FQueues.Current) then 
    Break; 
    LookedForSteals := True; 
end 

अब हम एक परीक्षण वातावरण की जरूरत है चोरी को देखने के लिए:

program WatchStealingTasks; 

{$APPTYPE CONSOLE} 
{$R *.res} 

uses 
    Winapi.Windows, 
    System.SysUtils, 
    System.Threading, 
    System.Classes, 
    System.Math; 

procedure OutputDebugStr(const AStr: string); overload; 
begin 
    OutputDebugString(PChar(AStr)); 
end; 

procedure OutputDebugStr(const AFormat: string; const AParams: array of const); overload; 
begin 
    OutputDebugStr(Format(AFormat, AParams)); 
end; 

function CreateInnerTask(AThreadId: Cardinal; AValue: Integer; APool: TThreadPool): ITask; 
begin 
    Result := TTask.Run(
     procedure 
    begin 
     Sleep(AValue); 
     if AThreadId <> TThread.CurrentThread.ThreadID 
     then 
     OutputDebugStr('[%d] executed stolen task from [%d]', [TThread.CurrentThread.ThreadID, AThreadId]) 
     else 
     OutputDebugStr('[%d] executed task', [TThread.CurrentThread.ThreadID]); 
    end, APool); 
end; 

function CreateTask(AValue: Integer; APool: TThreadPool): ITask; 
begin 
    Result := TTask.Run(
    procedure 
    var 
     LIdx: Integer; 
     LTasks: TArray<ITask>; 
    begin 
     // Create three inner tasks per task 
     SetLength(LTasks, 3); 
     for LIdx := Low(LTasks) to High(LTasks) do 
     begin 
      LTasks[LIdx] := CreateInnerTask(TThread.CurrentThread.ThreadID, AValue, APool); 
     end; 
     OutputDebugStr('[%d] waiting for tasks completion', [TThread.CurrentThread.ThreadID]); 
     TTask.WaitForAll(LTasks); 
     OutputDebugStr('[%d] task finished', [TThread.CurrentThread.ThreadID]); 
    end, APool); 
end; 

procedure Test; 
var 
    LPool: TThreadPool; 
    LIdx: Integer; 
    LTasks: TArray<ITask>; 
begin 
    OutputDebugStr('Test started'); 
    try 
    LPool := TThreadPool.Create; 
    try 
     // Create three tasks 
     SetLength(LTasks, 3); 
     for LIdx := Low(LTasks) to High(LTasks) do 
     begin 
      // Let's put some heavy work (200ms) on the first tasks shoulder 
      // and the other tasks just some light work (20ms) to do 
      LTasks[LIdx] := CreateTask(IfThen(LIdx = 0, 200, 20), LPool); 
     end; 
     TTask.WaitForAll(LTasks); 
    finally 
     LPool.Free; 
    end; 
    finally 
    OutputDebugStr('Test completed'); 
    end; 
end; 

begin 
    try 
    Test; 
    except 
    on E: Exception do 
     Writeln(E.ClassName, ': ', E.Message); 
    end; 
    ReadLn; 

end. 

और डीबग लॉग

 
Debug-Ausgabe: Test started Prozess WatchStealingTasks.exe (4532) 
Thread-Start: Thread-ID: 2104. Prozess WatchStealingTasks.exe (4532) 
Thread-Start: Thread-ID: 2188. Prozess WatchStealingTasks.exe (4532) 
Thread-Start: Thread-ID: 4948. Prozess WatchStealingTasks.exe (4532) 
Debug-Ausgabe: [2188] waiting for tasks completion Prozess WatchStealingTasks.exe (4532) 
Debug-Ausgabe: [2104] waiting for tasks completion Prozess WatchStealingTasks.exe (4532) 
Thread-Start: Thread-ID: 2212. Prozess WatchStealingTasks.exe (4532) 
Debug-Ausgabe: [4948] waiting for tasks completion Prozess WatchStealingTasks.exe (4532) 
Debug-Ausgabe: [2188] executed task Prozess WatchStealingTasks.exe (4532) 
Debug-Ausgabe: [4948] executed task Prozess WatchStealingTasks.exe (4532) 
Debug-Ausgabe: [2188] executed task Prozess WatchStealingTasks.exe (4532) 
Debug-Ausgabe: [4948] executed task Prozess WatchStealingTasks.exe (4532) 
Debug-Ausgabe: [2188] executed task Prozess WatchStealingTasks.exe (4532) 
Debug-Ausgabe: [2188] task finished Prozess WatchStealingTasks.exe (4532) 
Debug-Ausgabe: [4948] executed task Prozess WatchStealingTasks.exe (4532) 
Debug-Ausgabe: [4948] task finished Prozess WatchStealingTasks.exe (4532) 
Debug-Ausgabe: [2104] executed task Prozess WatchStealingTasks.exe (4532) 
Debug-Ausgabe: [2188] executed stolen task from [2104] Prozess WatchStealingTasks.exe (4532) 
Debug-Ausgabe: [4948] executed stolen task from [2104] Prozess WatchStealingTasks.exe (4532) 
Debug-Ausgabe: [2104] task finished Prozess WatchStealingTasks.exe (4532) 
Debug-Ausgabe: Thread Exiting: 2188 Prozess WatchStealingTasks.exe (4532) 
Debug-Ausgabe: Thread Exiting: 4948 Prozess WatchStealingTasks.exe (4532) 
Thread-Ende: Thread-ID: 4948. Prozess WatchStealingTasks.exe (4532) 
Thread-Ende: Thread-ID: 2188. Prozess WatchStealingTasks.exe (4532) 
Thread-Ende: Thread-ID: 2212. Prozess WatchStealingTasks.exe (4532) 

ठीक है, चोरी कार्यकर्ता धागे के किसी भी संख्या के साथ अब काम करना चाहिए , तो सब कुछ ठीक है?

नहीं

इस छोटे से परीक्षण आवेदन का अंत आ नहीं होगा, क्योंकि अब यह थ्रेड पूल का नाशक के अंदर जमा। पिछले कार्यकर्ता धागा

procedure TThreadPool.TQueueWorkerThread.Execute; 

... 

if ThreadPool.FWorkerThreadCount = 1 then 
begin 
    // it is the last thread after all tasks executed, but 
    // FQueuedRequestCount is still on 7 - WTF 
    if ThreadPool.FQueuedRequestCount = 0 then 
    begin 

एक और बग की वजह से समाप्त नहीं होगा यहाँ ठीक करने के लिए ... क्योंकि जब तब कार्य अब आप के लिए इंतजार कर रहे हैं के सभी Task.WaitForAll के साथ कार्य के लिए इंतज़ार कर, आंतरिक रूप से फांसी दे दी गई लेकिन कमी नहीं होगी FQueuedRequestCount

फिक्सिंग कि

function TThreadPool.TryRemoveWorkItem(const WorkerData: IThreadPoolWorkItem): Boolean; 
begin 
    Result := (QueueThread <> nil) and (QueueThread.WorkQueue <> nil); 
    if Result then 
    Result := QueueThread.WorkQueue.LocalFindAndRemove(WorkerData); 
    {$IFDEF USE_BUGFIX} 
    if Result then 
    DecWorkRequestCount; 
    {$ENDIF} 
end; 

और अब यह चलाता है जैसे कि यह एक ही बार में किया जाना चाहिए।


अद्यतन

रूप उवे द्वारा एक टिप्पणी हम भी परीक्षण के लिए तय System.Generics.Collections.TArray.Copy<T>

class procedure TArray.Copy<T>(const Source, Destination: array of T; SourceIndex, DestIndex, Count: NativeInt); 
{$IFDEF USE_BUGFIX} 
begin 
    CheckArrays(Pointer(@Source[0]), Pointer(@Destination[0]), SourceIndex, Length(Source), DestIndex, Length(Destination), Count); 
    if IsManagedType(T) then 
    System.CopyArray(Pointer(@Destination[DestIndex]), Pointer(@Source[SourceIndex]), TypeInfo(T), Count) 
    else 
    System.Move(Pointer(@Source[SourceIndex])^,Pointer(@Destination[DestIndex])^, Count * SizeOf(T)); 
end; 
{$ELSE} 
begin 
    CheckArrays(Pointer(@Source[0]), Pointer(@Destination[0]), SourceIndex, Length(Source), DestIndex, Length(Destination), Count); 
    if IsManagedType(T) then 
    System.CopyArray(Pointer(@Destination[SourceIndex]), Pointer(@Source[SourceIndex]), TypeInfo(T), Count) 
    else 
    System.Move(Pointer(@Destination[SourceIndex])^, Pointer(@Source[SourceIndex])^, Count * SizeOf(T)); 
end; 
{$ENDIF} 

एक साधारण जांच को ठीक करने की जरूरत है:

procedure TestArrayCopy; 
var 
    LArr1, LArr2: TArray<Integer>; 
begin 
    LArr1 := TArray<Integer>.Create(10, 11, 12, 13); 
    LArr2 := TArray<Integer>.Create(20, 21); 
    // copy the last 2 elements from LArr1 to LArr2 
    TArray.Copy<Integer>(LArr1, LArr2, 2, 0, 2); 
end; 
  • XE7 साथ आप XE7 Update1 साथ एक अपवाद
  • मिल जाएगा आप
     
    LArr1 = (10, 11, 0, 0) 
    LArr2 = (20, 21) 
    
  • उस के साथ
  • मिल ऊपर समस्याओं का समाधान कर
     
    LArr1 = (10, 11, 12, 13) 
    LArr2 = (12, 13) 
    
+2

इस सब से दूर लेना यह है कि कोड वर्तमान में भरोसेमंद नहीं है। एम्बा कुछ डेवलपर्स को किराए पर लेने जा रही है जिनके पास सही बहु-थ्रेडेड कोड लिखने की क्षमता है? यह फिर से 'TMonitor' है। –

+1

हां सबसे अधिक समय हम अच्छे विचारों को खराब कार्यान्वित करेंगे और वास्तव में गलत प्लेटफॉर्म पर परीक्षण या परीक्षण नहीं करेंगे या सिर्फ "यह संकलित" परीक्षण करेंगे। –

4

यह System.CopyArray में कोई बग नहीं है। डिज़ाइन द्वारा यह केवल प्रबंधित प्रकारों का समर्थन करता है। बग वास्तव में TArray.Copy<T> में है। T एक प्रबंधित प्रकार है या नहीं, इस पर भेदभाव किए बिना System.CopyArray पर कॉल करने में यह गलत है।

हालांकि, XE7 अद्यतन 1 से TArray.Copy<T> का नवीनतम संस्करण आपके द्वारा वर्णित समस्या से पीड़ित नहीं होता है।कोड इस तरह दिखता है:

class procedure TArray.Copy<T>(const Source, Destination: array of T; 
    SourceIndex, DestIndex, Count: NativeInt); 
begin 
    CheckArrays(Pointer(@Source[0]), Pointer(@Destination[0]), SourceIndex, 
    Length(Source), DestIndex, Length(Destination), Count); 
    if IsManagedType(T) then 
    System.CopyArray(Pointer(@Destination[SourceIndex]), 
     Pointer(@Source[SourceIndex]), TypeInfo(T), Count) 
    else 
    System.Move(Pointer(@Destination[SourceIndex])^, Pointer(@Source[SourceIndex])^, 
     Count * SizeOf(T)); 
end; 

जब तक मैं अपने विश्लेषण में गलत है, तो आप बस System.CopyArray के साथ समस्याओं को हल करने के लिए अद्यतन 1 लागू करने के लिए की जरूरत है।


लेकिन जैसा कि यूवे नीचे टिप्पणियों में इंगित करता है, यह कोड अभी भी फर्जी है। यह SourceIndex गलती से उपयोग करता है जहां DestIndex का उपयोग किया जाना चाहिए। और स्रोत और गंतव्य पैरामीटर गलत क्रम में पारित कर दिए जाते हैं। एक आश्चर्य भी करता है कि लेखक ने Destination[SourceIndex] की बजाय Pointer(@Destination[SourceIndex])^ क्यों लिखा। मुझे यह पूरी स्थिति बहुत निराशाजनक लगता है। Embarcadero इस तरह की भयानक गुणवत्ता का कोड कैसे जारी कर सकते हैं?


उपरोक्त से अधिक गहराई TSparseArray<T> के साथ समस्याएं हैं। कौन इस तरह दिखता है:

function TSparseArray<T>.Add(const Item: T): Integer; 
var 
    I: Integer; 
    LArray, NewArray: TArray<T>; 
begin 
    while True do 
    begin 
    LArray := FArray; 
    TMonitor.Enter(FLock); 
    try 
     for I := 0 to Length(LArray) - 1 do 
     begin 
     if LArray[I] = nil then 
     begin 
      FArray[I] := Item; 
      Exit(I); 
     end else if I = Length(LArray) - 1 then 
     begin 
      if LArray <> FArray then 
      Continue; 
      SetLength(NewArray, Length(LArray) * 2); 
      TArray.Copy<T>(LArray, NewArray, I + 1); 
      NewArray[I + 1] := Item; 
      Exit(I + 1); 
     end; 
     end; 
    finally 
     TMonitor.Exit(FLock); 
    end; 
    end; 
end; 

केवल समय FArray आरंभ नहीं हो जाता TSparseArray<T> निर्माता है। इसका मतलब है कि अगर सरणी भर जाती है, तो आइटम जोड़े और खो जाते हैं। संभवतः I = Length(LArray) - 1FArray की लंबाई बढ़ाने और नई वस्तु को कैप्चर करने के लिए है। हालांकि, ध्यान दें कि TSparseArray<T>FArrayCurrent संपत्ति के माध्यम से खुलासा करता है। और यह एक्सपोजर लॉक द्वारा संरक्षित नहीं है। इसलिए, मैं नहीं देख सकता कि यह वर्ग FArray पूर्ण हो जाने पर किसी भी उपयोगी तरीके से कैसे व्यवहार कर सकता है।

मेरा सुझाव है कि आप एक उदाहरण बनाते हैं जहां FArray पूर्ण रूप से प्रदर्शित होता है कि जो आइटम जोड़े गए हैं वे खो गए हैं। यह दर्शाते हुए एक बग रिपोर्ट सबमिट करें, और इस प्रश्न से लिंक करें।

+0

यह * वास्तव में * 'TSparseArray ' ठीक नहीं करता है। अपवाद अब और नहीं उठाएगा और सबकुछ * ठीक लगता है *, लेकिन अजीब और बेकार प्रतिलिपि अभी भी वहां है और खराब गंध छोड़ देता है। मैं कोड में और अधिक खोदने की कोशिश करूंगा क्यों यह सभी प्रतिलिपि भाग के बिना काम करता है। –

+0

बीटीडब्ल्यू आप वास्तव में सही हैं, कि 'System.CopyArray' दस्तावेज के रूप में काम करता है। मेरी गलती 'TArray.Copy ' के लेखक के समान थी - दस्तावेज़ों को जानने/पढ़ने के बजाय विश्वास/मानना: ओ) –

+2

@ सिरुफू मुझे लगता है कि यह इससे कहीं अधिक गहरा है। आपके प्रश्न में विकल्प 2 का उपयोग करने के लिए पर्याप्त नहीं है क्योंकि 'फेयर्रे' को लॉक के बिना सार्वजनिक रूप से उजागर किया जाता है। मुझे लगता है कि हमारे लिए आवेदन करने के लिए कोई आसान फिक्स नहीं है, कम से कम नहीं क्योंकि हम वास्तव में इस वर्ग के डिजाइन को नहीं जान सकते हैं।व्यक्तिगत रूप से मैं उम्मीद करता हूं कि इस नई थ्रेडिंग लाइब्रेरी को ठीक करने के लिए एम्बा को लगभग 5 साल लगें, जैसे कि उन्हें 'टीएमओनिटर' को ठीक करने में काफी समय लगेगा। मुझे विश्वास नहीं है कि उनके पास एक स्वच्छ थ्रेडिंग लाइब्रेरी लिखने का कौशल है। –

संबंधित मुद्दे