Click here to Skip to main content
Rate this: bad
good
Please Sign up or sign in to vote.
Hello,
I am using SyncLock to synchronize threads access to the returnedDataQueue object, the queue is public property, I know it's not a good practice to lock public variables but I don't know how to accomplish my goal without doing it.
I have 2 threads accessing the same queue, the first thread (ListenerThread) listens to TCP transactions that comes from the server, then it queues them into the returnedDataQueue, the second thread dequeues from the returnedDataQueue.
 
The Listener Thread:
------------
Public Class ConnectionHandler
    Inherits TcpClient
 
    Public Event dataArrived()
 
    Private listenThread As Thread
 
    Private _returnedDataQueue As Queue
    Public Property returnedDataQueue() As Queue
        Get
            Return Queue.Synchronized(_returnedDataQueue)
        End Get
        Set(ByVal value As Queue)
            _returnedDataQueue = value
        End Set
    End Property
 
    Sub New(ByVal ip As String, ByVal port As Integer)
 
        returnedDataQueue = New Queue
        listenThread = New Thread(AddressOf listen) 
        listenThread.Name = "ListenThread"
        listenThread.SetApartmentState(Threading.ApartmentState.STA) 
        listenThread.Start()
    End Sub
 
    Public Sub listen()
        Dim networkStream As NetworkStream
        Do While Not MyBase.Connected
            Me.connect()
        Loop
 
        Dim strData As String
        Do While MyBase.Connected
            If networkStream.CanRead Then
                Dim bytes(MyBase.ReceiveBufferSize) As Byte
                networkStream.Read(bytes, 0, CInt(MyBase.ReceiveBufferSize))
                ' Read the NetworkStream into a byte buffer.
                strData = Encoding.ASCII.GetString(bytes).Trim
 
                SyncLock returnedDataQueue.SyncRoot
                    returnedDataQueue.Enqueue(strData)
                End SyncLock
 
                RaiseEvent dataArrived()
            End If
        Loop
    End Sub
End Class
 
In the main thread, I handle the dataArrived() events:
Dequeu Thread:
-------------
 
Public Class MyService
    Inherits System.ServiceProcess.ServiceBase
 
    Private WithEvents objConnectionHandler As ConnectionHandler
    Private objUdpClient As UDPMulticaster
	
    Protected Overrides Sub OnStart(ByVal args() As String)
        objUdpClient = New UDPMulticaster(settings.MulticastIP, settings.MulticastPort, settings.MulticastPortEP)
        objConnectionHandler = New ConnectionHandler(settings.IP, settings.Port)
    End Sub
 
    Private Sub objConnectionHandler_dataArrived() Handles objConnectionHandler.dataArrived
        Try
            SyncLock objConnectionHandler.returnedDataQueue.SyncRoot
                While objConnectionHandler.returnedDataQueue.Count > 0
                    Dim strTemp As String = objConnectionHandler.returnedDataQueue.Dequeue
                    Dim success As Boolean = CBool(wcfService.ProcessTransaction(strTemp))
                    If success Then
                        Dim seq As Integer = CInt(wcfService.getSequenceNumber())
                        objUdpClient.multicastToNetwork(strTemp, seq)
                    End If
                End While
            End SyncLock
        Catch ex As Exception
             'Write Log
        End Try
    End Sub
End Class
 
Suddenly, the ListenThread stops working, I don't get any errors, it looks that the code is not executing anymore and the thread is waiting for something, after running the code for couple of days on a busy system, I believe the Dequeue Thread locks the returnedDataQueue for long time, then the listen thread won't be able to queue any new transactions!
 
Is there anything wrong with the code? is there any alternative approach to prevent the threads from accessing the queue simultaneously?
 
Thanks,
Sam
Posted 20-Jun-12 6:20am
Edited 20-Jun-12 6:22am
v2

1 solution

Rate this: bad
good
Please Sign up or sign in to vote.

Solution 1

This design is a bit hard to follow. I would advise to use a bit different approach, without using async API. Instead, the application which listens for new connections and performs data exchange on the network (reading/writing from/to network stream or reading/writing form/to a remote socket) should do it in separate threads, both using blocking call. You see, the sockets themselves serve as a synchronization primitive, you can even implement a lock or a mutex using pure socket API. (Historically, threads were initially developed as IPC objects, not for network communications.) However, you will need lock to share some collection where you store the remote sockets or remote TcpClient instances.
 
Due to simplicity of such primarily sequential design, you have no danger of deadlock while having less code and more control. You can find some ideas in my past answer:
Multple clients from same port Number[^].
 
As to the blocking queue, you can refer to my well-tested and explained implementation (sorry, it is C#):
Simple Blocking Queue for Thread Communication and Inter-thread Invocation[^].
 
—SA
  Permalink  
v2
Comments
losmac at 20-Jun-12 15:39pm
   
Wow! Maestro!
Sergey Alexandrovich Kryukov at 20-Jun-12 15:42pm
   
Thank you so much, Maciej.
--SA

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)

  Print Answers RSS
0 OriginalGriff 195
1 ProgramFOX 130
2 Maciej Los 105
3 Sergey Alexandrovich Kryukov 85
4 Afzaal Ahmad Zeeshan 82
0 OriginalGriff 6,564
1 Sergey Alexandrovich Kryukov 6,048
2 DamithSL 5,228
3 Manas Bhardwaj 4,717
4 Maciej Los 4,150


Advertise | Privacy | Mobile
Web02 | 2.8.1411022.1 | Last Updated 20 Jun 2012
Copyright © CodeProject, 1999-2014
All Rights Reserved. Terms of Service
Layout: fixed | fluid

CodeProject, 503-250 Ferrand Drive Toronto Ontario, M3C 3G8 Canada +1 416-849-8900 x 100