Unexpected data loss during static Queue.Enqueue in a multi-threaded process

Question

private Queue _queueObject = new Queue ();

private static queue _queueItem = new Queue ();

private static int allowEntryCount = 0;

private int allowThreadEntry = 0;

I have two queue variables as shown above.

public Camera(IVideoSource source, MotionDetector detector) { VideoSource = source; _motionDetector = detector; VideoSource.NewFrame += VideoNewFrame; MainForm.OnRegisterClickedEvent += new MainForm.RegisterClickedEventHandler(MainForm_OnRegisterClickedEvent); MainForm.OnReceiveMultipleFrameEvent += new MainForm.ReceiveMultipleFrameEventHandler(MainForm_OnReceiveMultipleFrameEvent); } 

I have a Camera class, and shown above, is part of the constructor implementation. The video source always listens for the VideoNewFrame event; The code shown below is one segment of code in VideoNewFrame.

 FrameObjectElement frameObj = new FrameObjectElement(); frameObj.cameraID = CW.Camobject.id; frameObj.cameraTag = _FPGAFrameCount / 2; frameObj.FirstFrameBuffer = BitmapToRgbValues(twoframe_arg.GetFirstBitmap(352, 288)); frameObj.SecondFrameBuffer = BitmapToRgbValues(twoframe_arg.GetSecondBitmap(352, 288)); if (_queueObject.Count > 5) _queueObject.Clear(); _queueObject.Enqueue(frameObj); if (allowThreadEntry == permitEntryCount && isClear) { if (_queueObject.Count != 0) { lock (this) { _queueItem.Enqueue(_queueObject.Peek()); } Debug.WriteLine("Thread ID: " + Thread.CurrentThread.ManagedThreadId.ToString() + " - " + _queueObject.Count.ToString() + " queue in QueueObject : Camera ID : " + _queueObject.Peek().cameraID.ToString() + " : Camera Tag : " + _queueObject.Peek().cameraTag.ToString() + " : Queue item count : " + _queueItem.Count.ToString()); _queueObject.Dequeue(); if (_queueItem.Count == numberOfCamera && isAllow) { CurrentID = CW.Camobject.id; isAllow = false; } allowThreadEntry++; if (_queueItem.Count == numberOfCamera) { if (CurrentID == CW.Camobject.id) { isClear = false; //allowEntry = false; //Debug.WriteLine("-- Count: " + allowThreadEntry.ToString() + " --"); foreach (FrameObjectElement foE in _queueItem) { Debug.WriteLine("Current Camera ID: " + CW.Camobject.id + " : Camera ID : " + foE.cameraID.ToString() + " : Camera Tag : " + foE.cameraTag.ToString() + " :"); } MultipleFrameEventArgs newMul = new MultipleFrameEventArgs(); newMul.itemObj = _queueItem; if (OnMultupleFrameEvent != null) OnMultupleFrameEvent(newMul); _queueItem.Clear(); isAllow = true; isClear = true; Debug.WriteLine("Queue item count: " + _queueItem.Count.ToString() + " : isClear : " + isClear.ToString()); } } } } 

Basically, what I'm trying to achieve here is to collect the frame identifier, label, its first and second frames, and then save it in an object (struct FrameObjectElement). Each collection of 2 frames will represent 1 camera tag, so the meaning of its role is here. Then the frameobject is queued in '_queueObject'. Then I would have the condition "if (allowThreadEntry == allowEntryCount)". So, what is done here, every time this function is available, "allowThreadEntry" will increase, and "allowCountEntry" will remain the same. Then this function will continue and complete the first element in '_queueObject' in '_queueItem', and as soon as the desired counter _queueItem is satisfied, it will raise a signal to another class. This class will respond back by raising the signal that was previously selected by the camera class, as shown "MainForm.OnReceiveMultipleFrameEvent + = new MainForm.ReceiveMultipleFrameEvent (MainForm_OnReceiveMultipleFrameEvent)."

void MainForm_OnReceiveMultipleFrameEvent (MainForm.ReceiveMultpleFrameEventArgs e) {allowEntryCount ++; }

After receiving the signal, allowEntryCount will increase, which will allow access to the function again. Why I do this because this class is created depends on how many camera objects I have. If I had 11 cameras, I would have 11 workThread handling this class. I queue my frames in a non-static queue and collect their first element in a static queue, which must be transferred for my other process. The problem I encountered is as follows:

============================== Graph: 1760 =================== ==============

Number of Queue Elements: 0: isClear: True

Stream ID: 17 - 3 queue in QueueObject: Camera ID: 3: Camera tag: 3372: Number of queue elements: 1

Stream ID: 24 - 6 queue in QueueObject: Camera ID: 10: Camera label: 4367: Number of queue elements: 2

Stream ID: 23 - 5 queue in QueueObject: Camera ID: 9: Camera tag: 4415: Number of items in the queue: 3

Stream ID: 19 - 1 queue in QueueObject: Camera ID: 5: Camera label: 4108: Number of queue elements: 4

Stream ID: 20 - 5 queue in QueueObject: Camera ID: 6: Camera tag: 3768: Number of items in the queue: 5

Stream ID: 14 ​​- 1 queue in QueueObject: Camera ID: 0: Camera label: 2837: number of items in the queue: 6

Stream ID: 21 - 1 queue in QueueObject: Camera ID: 7: Camera Tag: 3246: Number of items in the queue: 7

Stream ID: 16 - 1 queue in QueueObject: Camera ID: 2: Camera tag: 3552: number of items in the queue: 8

Stream ID: 18 - 6 queue in QueueObject: Camera ID: 4: Camera tag: 3117: Number of items in the queue: 9

Stream ID: 15 - 3 queues in QueueObject: Camera ID: 1: Camera Label: 2315: Number of Queue Elements: 10

Stream ID: 22 - 4 queue in QueueObject: Camera ID: 8: Camera tag: 4853: number of items in the queue: 11

Current camera ID: 8: Camera ID: 3: Camera label: 3372:

Current Camera ID: 8: Camera ID: 10: Camera Label: 4367:

Current camera ID: 8: Camera ID: 9: Camera label: 4415:

Current camera ID: 8: Camera ID: 5: Camera label: 4108:

Current Camera ID: 8: Camera ID: 6: Camera Tag: 3768:

Current camera ID: 8: Camera ID: 0: Camera label: 2837:

Current camera ID: 8: Camera ID: 7: Camera label: 3246:

Current Camera ID: 8: Camera ID: 2: Camera Label: 3552:

Current camera ID: 8: Camera ID: 4: Camera label: 3117:

Current Camera ID: 8: Camera ID: 1: Camera Label: 2315:

Current camera ID: 8: Camera ID: 8: Camera label: 4853:

============================== Graph: 1761 =================== ==============

Number of Queue Elements: 0: isClear: True

Stream ID: 14 ​​- 1 queue in QueueObject: Camera ID: 0: Camera label: 2838: Number of queue elements: 1

Stream ID: 16 - 1 queue in QueueObject: Camera ID: 2: Camera tag: 3553: Number of queue elements: 2

Stream ID: 21 - 1 queue in QueueObject: Camera ID: 7: Camera Tag: 3247: Number of items in the queue: 3

Stream ID: 24 - 1 queue in QueueObject: Camera ID: 10: Camera tag: 4374: Number of items in the queue: 4

Thread ID: 23 - 6 queue in QueueObject: Camera ID: 9: Camera tag: 4416: number of items in the queue: 5

Stream ID: 17 - 4 queues in QueueObject: Camera ID: 3: Camera Tag: 3373: Number of items in the queue: 7

Stream ID: 15 - 3 queues in QueueObject: Camera ID: 1: Camera label: 2316: Number of items in the queue: 7

Stream ID: 18 - 6 queue in QueueObject: Camera ID: 4: Camera tag: 3118: Number of items in the queue: 8

Stream ID: 20 - 6 queue in QueueObject: Camera ID: 6: Camera tag: 3769: Number of items in the queue: 9

Stream ID: 22 - 4 queue in QueueObject: Camera ID: 8: Camera tag: 4854: number of queue elements: 10

I should have a different count in '_queueItem', since each object created can only be accessed once in this segment, thus letting me know that their item will be placed in '_queueItem'. But, unfortunately, somehow, after the program lasts for some time, such a situation will appear, as shown above. Either I apply a lock or not in this part of '_queueItem.Enqueue (_queueObject.Peek ()); I will still have a problem. May I know where I made a mistake?

Answer 1

You say that the queue is static , but you blocked the instance:

 lock (this) { _queueItem.Enqueue(_queueObject.Peek()); } 

If you have multiple instances, this means that each of them blocks independently . A better approach would be to have a dedicated static lock object and lock it. Perhaps you can fool:

 lock (_queueItem) { _queueItem.Enqueue(_queueObject.Peek()); } 

if _queueItem never reassigned, but the safest approach:

 static readonly object lockObj = new object(); lock (lockObj) { _queueItem.Enqueue(_queueObject.Peek()); } 

Note that all access to the queue must be synchronized and must all use the same lock object.

You may be able to reduce the number of conflicts if you talk with the two queues separately, but try to avoid using locks nested in this scenario, as this can lead to locks if this is done poorly; for example, to look from the instance queue and queue in the static queue, you can use:

 object item; lock(instanceLock) { item = _queueObject.Peek(); } lock(staticLock) { _queueItem.Enqueue(item); } 

Note that even simple things like .Count should be synchronized and ideally double checked (you cannot check the account at the beginning of the method, and then, assuming there is still something that can be deleted, if only you keep the lock for the entire duration). Your code repeats using .Count - so please be very careful. .Count is temporary: as soon as you read it, if you refuse to block, you should assume that this is already wrong.

Answer 2

Locking with this is evil. Check out some links:

Why is blocking (this) {...} bad?

http://haacked.com/archive/2005/04/12/neverlockthis.aspx

Use always the selected object, for example:

 private static object _queueLock = new object(); 

Answer 3

Why don't you let Queue handle the lock? Use ConcurrentQueue<T> , which is synchronized internally.

Answer 4

Do not use lock (this) - see here . If you are sharing a queue between multiple instances of a class, you need to block the static object common to all instances. Currently, each instance of the class has its own lock, so it will ignore any locks used by other instances.