Link to home
Start Free TrialLog in
Avatar of John500
John500Flag for United States of America

asked on

Question for --> itsmeandnobodyelse: shared mutex for multi threaded socket application

itsmeandnobodyelse,

Please note again, the application I have starts off looking like this below:


int APIENTRY _tWinMain(HINSTANCE hInstance,
           HINSTANCE hPrevInstance,
           LPTSTR lpCmdLine,
                                int       nCmdShow)
{


     System::Threading::Thread::CurrentThread->ApartmentState = System::Threading::ApartmentState::STA;
     
     // Start the application
     Application::Run(new Form1());

     
     return 0;
}

When this application runs, three threads are off and running and doing their job of collecting data.  It's a done deal, no more code writing in that area - I won't go there again, done.

At least I want to believe that I don't have to touch that code other than to devise a way to capture data from each thread and get it into a circular queue where I can make a decision on whether the data is good enough to land the aircraft on final.  That aircraft on final is identified by one of those threads.  

Therefore, that thread is a driving force because it tells me with each packet received from the socket who is claiming to be the aircraft on final, second, third, fourth etc.  So then, this thread would seemingly become the thread responsible for dictating or populating the structures of the queue.   As those structures are placed into the queue, information would also be pulled from the other threads to update one and only one structure ---->  the head of the queue.

How can I do this in the simplest fashion considering I have all this preexisting code in place that I _do_ _not_ want to revamp if at possible.  In other words, I'm already creating threads like:

WindThread = new Thread(new ThreadStart(this, WindThreadProc));
WindThread->Start();

void Form1::WindThreadProc()
{
  ...
  ...
   ReadWindSocket()
  ...
}

I don't want to have to go back and reinvent a wheel I've already created invented.  I want to take the data read from each socket thread and share it globably.

How am I going to do this ?

Thanks  !!

Avatar of itsmeandnobodyelse
itsmeandnobodyelse
Flag of Germany image

Hi John,

thanks for the points again.

You don't need to announce the threads for me personally. Actually, in EE each question is free for any expert to participate. And it is a good chance that you might get ideas from other inputs as well.

Here are the links to the threads preceeding that question http:Q_21828294.html and http:Q_21830954.html.

That doesn't mean that I don't want to help you further.


>>>>      Application::Run(new Form1());

Yes, you are using a framework and we can move all code so far so that it fits to the framework.

Unfortunately, I have only little experience with VC.NET and Windows Forms  (I purchased the Visual Studio 2005 Prof. Edition but didn't install yet). So, I would need some information like the one above to be able to help you.

>>>> Form1

What's the purpose of the form (or how looks it like)? Do you show the results of the simulation in that form?

Generally, the tower application is designed is a server application. That means it should run it's own infinite loop where it checks periodically for incoming requests or new notifications and running the landing queue.

       ...
       while (true)
       {
             if (getExternalMessages(...))
             {
                 int ret = handleExternalMessages();
                 if (ret == -1)
                     break;    // break the loop if quit message
             }
             if (getNotifications(...))
                 handleNotifications();
             if (m_landingQueue.size() > 0)
             {
                   runLandingQueue();
             }
             Sleep(50);   // sleep a little while to give other threads a chance
       }  


With the .NET framework and the call "Application::Run(new Form1());" you already got an infinite loop. However, it isn't a server loop but a message loop which purpose is to drive the dialog defined by Form1. We could setup a timer within that dialog that was invoked any 50 milliseconds and put the code from the while loop above into the TimerProc. However, I would recommend against it. The GUI is pretty suitable for visualizing the results of the simulation. Moreover, it could play the role of the external client(s) we discussed before. Both - visualizing and control - shouldn't be mixed up with the server job of running the simulation.

I could think of two solutions: (1) you simply start only one thread that runs the tower in the way I outlined before.  Then, you send requests from the GUI to the tower via sockets announcing new airplanes, changing wind parameters and so on. Periodically, the tower thread sends all status info to the GUI where it was showed in a result wind (of course you could draw images and make an animation if you like better). The status info was sent by socket as well.  The tower server creates additional threads for any airplane. The GUI can send requests to each airplane via socket (use a new port for any plane). (2) You start a new process from the GUI rather than creating threads. Then, the new process would be a plain Win32 application rather than a .NET appliciation (what makes most things easier).


>>>> I don't want to have to go back and reinvent a wheel I've already created invented.

Don't think reinventing the wheel is the main problem. The framework's interface is suitable to run a client application. We need to write a client-server application and the server part is the main part. Here .Net doesn't help. Sockets and threads don't need much code. Actually, the .NET interface for sockets and threads wouldn't give much advantage before the native interface if not even preventing from doing the simple.

Regards, Alex




Avatar of John500

ASKER

>> thanks for the points again.

No problem, my pleasure.  

>>  What's the purpose of the form (or how looks it like)?

With a .Net Form Application, it looks just like a Visual Basic Form would look like in VS 6.0.  I have created three group boxes for threads that are running.  Each thread starts a socket and reads from the socket. The data collected is sent to the appropriate group box.  I have every thing I need but safe global sharing technique.

Alex, I still think you are making this more difficult than it is.  Or, I still haven't caught your idea yet.  In my mind there will absolutely be no more sockets, none, and no client/server activity at all.

As it is right now each thread does two things:

1)  displays it's own data to the Form group box
2)  stores it's data in a global array i.e.

extern string WindArray[5];     // stores four elements of wind
extern string RadarArray[3];    // stores two elements - aircraft side number and distance
extern string AircraftArray[4];  // stores three elements - pilot, side number, final line-up position

Every thread runs from within the Form.cpp but all the code that supports each thread is located in separate cpps:

Wind.cpp
Radar.cpp
Aircraft.cpp

Hence, within the Form.cpp I declare each array so that it can be accessed from each cpp in the project.  This of course won't work once I have these threads competing for the same array.  So then, I have devised the structures that you have already seen which I want to use in a queue.

I still say that the Aircraft thread (which gets final line-up info) should be the thread that creates each new structure for the queue as it receives indication from a socket/packet that there is in fact an additional bird.  This additional bird is given a projected line-up number from the server that sent it.  They can't all be on final and I _don't_ make that decision.  I only perfrom a cross-check to allow the landing.  Hence, the Aircraft thread drives the whole application by virtue of the fact that it knows how many birds are coming in for a landing.

I need to use this thread to create queue elements and also to determine if the landing will take place based on info from the other elements.  Tell me you see what I"m getting at here and that it doesn't need to be any more complex than this.  I see the need for an additional header and or cpp that provides the mutex code and queue.  The Form.cpp will call whatever it needs and the Form is triggered by the Aircraft thread each time it gets a new aircraft.

Standing by
 
>>>> the Aircraft thread (which gets final line-up info) should be the thread
>>>> that creates each new structure for the queue

Actually, it should be the Tower thread that should run the LandingQueue. The Aircraft thread could create the Aircraft objects and make the updates to Aircraft info.

>>>> Hence, the Aircraft thread drives the whole application by virtue of the fact that
>>>> it knows how many birds are coming in for a landing.

I strongly recommend against that concept. It would be the same as if all arriving aircrafts would run the tower...

You could create a new Tower thread *or* use a timer in your main application to run the tower. The latter can be invoked by calling SetTimer in the handler function of WM_INITDIALOG. The timer proc is a callback function that might be declared similar to the thread proc functions. It will be called - say any 10 milliseconds - and should reflect the changes made by the threads. Then it should actively control the landing queue You might consider to add a new result window in the main form where you could output the activities of the landing queue.

Regards, Alex
Avatar of John500

ASKER

Alex,

Ok.  I'll have another thread that looks like this:

TowerThread = new Thread(new ThreadStart(this, TowerThreadProc));
TowerThread->Start();

I'm guessing the guts of that thread will look like your post above:

void Form1::TowerThreadProc()
{

   while (true)
       {
             if (getExternalMessages(...))
             {
                 int ret = handleExternalMessages();
                 if (ret == -1)
                     break;    // break the loop if quit message
             }
             if (getNotifications(...))
                 handleNotifications();
             if (m_landingQueue.size() > 0)
             {
                   runLandingQueue();
             }
             Sleep(50);   // sleep a little while to give other threads a chance
       }  
}

Do you agree the next step is to write the getExternalMessages() function ?  Do you agree this function will utilize the mutex class ?  Do you agree the mutex class will look like this :

#include <mutex.h>


// mutex.h
#ifndef MUTEX_H
#define MUTEX_H

#include <windows.h>

class Mutex
{
private:
    CRITICAL_SECTION mutex;
public:
    Mutex() {  InitializeCriticalSection(&mutex);  }

    // enter critical section and block others threads or wait
    void enter() { EnterCriticalSection(&mutex); }
   
    // leave critical section and release any other thread blocked
    void leave() { LeaveCriticalSection(&mutex); }
};

#endif

#endif // MUTEX_H

If you agree with all this I have two questions:'

1)  Should the mutex class above compile as is ?  Do you see _anything_ that would make this thing not compile
2)  What would the definition of getExternalMessages() look like ?  Remember, I won't be using _any_ socket to socket communication.  These socket routines have already been written and collect their data accordingly.  It appears 'getExternalMessages()' would constitute the method of sharing the queue among all threads using the mutex class - right ?
>>>> void Form1::TowerThreadProc()

Ok, that is the dog wagging its tails ;-)

>>>> if (getExternalMessages(...))

getExternalMessages was intended to receive messages from external devices such as arriving aircrafts, wind, radar, ...

As you have an integrated client-server now, we could drop it and do all with (internal) notifications between threads.

It would be possible to read data from screen after the threads have updated their screen data. Though transferring data via screen is thread-safe, I suggest not to do so. Data read from screen might be incomplete (some controls were updated and some not). Furthermore, we shouldn't mixup GUI tier and Application tier.

To send notifications between objects that were handled in different threads, these objects need to know of each other. For simplicity, I would suggest to add a (public) member pointer of the Tower object to the Form1 class. Then all threads have access to the Tower after it was initially set (registered) by the Tower thread. All other threads then have access to Tower (Tower is a singleton) and could send their newly created objects (Aircraft, Radar and Wind objects) by using appropriate member functions of Tower. All objects - including Tower - periodically update their data in a thread-safe way by using a Mutex, and send notifications to the objects that need to handle the updates.  

You may wonder why I suddenly propose a Wind and Radar class. That's because of your Radar and Wind thread which should be able to run in a similar way as the Tower and Aircraft thread.

Tower, Aircraft, Wind and Radar should have a common baseclass, that could provide the notification functionality:

enum NotificationCode
{
    EMPTY_NOTIFICATION,
    PERMISSION_LANDING,
    PERMISSION_CANCELED,
    REQHEIGHT_CHANGED,
    APPROXWAITTIME_CHANGED,
    RUNWAY_CHANGED,
    REQUEST_GRANTED,
    REQUEST_DENIED,
    REQUEST_CANCELED,
    BREAK_LANDING,
    CONTINUE_LANDING,
    AIRCRAFTINFO_CHANGED
};

// forward declaration
class AirportObject;

class Notification
{
   NotificationCode m_nc;
   AirportObject*   m_from;
public:
    bool empty() { return m_nc == EMPTY-NOTIFICATION; }
    AirportObject*   getSender() { return m_from; }  
};

class AirportObject
{
protected:
     Mutex m_cs;  // put the mutex in the baseclass
     vector<Notification> m_nq;  // notification queue
public:
      void sendNotification(const  Notification& n)
      {
           m_nc.enter();
           m_nq.push_back(n);
           m_nc.leave();
      }
protected:
      Notification getNotification()
      {
           m_nc.enter();
           Notification n = {  EMPTY_NOTIFICATION, NULL };
           if (!m_nc.empty())
           {
                 n = m_nq[0];
                 m_nq.erase(0);
           }
           m_nc.leave();
           return n;
      }

};

You now should derive Tower, Aircraft, Wind, Radar from AirportObject.

Tower should get a vector of Aircraft pointers, a Wind and a Radar pointer member. We need to use pointers rather than objects as the objects were created in different threads. Hence, we need to create the objects while the associated members were empty (NULL pointers).

Aircraft, Wind, Radar should have a pointer to Tower. That pointer should be retrieved by Form1 where it is a (static) member as well. Note, the Tower member of Form1 was set by the Tower thread after creating the one and only Tower object. As the other threads were  created nearly same time than the Tower thread, the Form1 member could be NULL. You need to poll for a valid Tower pointer  because of that:

   void Form1::WindThreadProc()
   {
         Tower* pTower = NULL;
         while ((pTower = Form1::getTower()) == NULL)
               Sleep(10);

         Wind* pWind = new Wind(pTower);
         pTower->setWind(pWind);

         // run the infinite thread loop
         while (true)
         {
               ....
         }
   };


To continue ...

Regards, Alex
Avatar of dbkruger
dbkruger

I saw no reason to dynamically allocate the wind object and if there was then you should deallocate it. before leaving the above routine.

Is there a reason that the radar and wind have to be on a thread, or is that just the statement of the exercise? If they are continuously changing it makes more sense to update them every time you check the queue, thus negating the need for synchronization. It's important to follow the rules of the assignment, and if it calls for threads and synchronization, fine, but at the same time the end goal is to learn how to code correctly, and as I stated before, I believe the setup is not reasonable, which I assume to be because the problem itself is not well thought out. This is fine, many projects are that way, and one should be able to handle them anyway, but it's good to know exactly why it doesn't make sense, and what you could do better given the freedom to redesign the code.

cheers,
Dov
>>>> I saw no reason to dynamically allocate the wind object

Wind, Radar and Tower objects will be used in more than one thread. By using objects created on the stack you would need to synchronize objects deletion among all threads. Using dynamically created objects would allow to delete all objects after terminating all threads in the destructor of Tower by Form1 class. Another reason for creating the objects dynamically is to have same standards for all threads and the Aircraft objects need to be created dynamically in the Aircraft Thread.

>>>> Is there a reason that the radar and wind have to be on a thread

The thread concept (now) is that all extern devices like tower, aircraft(s), wind, and radar are driving their own threads. That is different to my first approach that uses threads for Tower and each single Aircraft, but has the advantage that all threads can be created at the beginning of the simulation application and have their own info controls in a groupbox in the GUI (Form1). The update of the form doesn't need further synchronisation. The update of the model objects (Tower, Aircraft, Wind, Radar) can be made thread-safe by using a mutex like in the sample above.

Regards, Alex

>Wind, Radar and Tower objects will be used in more than one thread.
You still need to deallocate what you allocate.
And if there are exactly one of each thing, Just pass them into each thread and don't deallocate them at all.
I shouldn't be seeing:

Wind* w = new ...
and no corresponding delete. While you could do the delete elsewhere, it's confusing, and highly error prone. This is why everyone encapsulates the new and delete in objects.

In any case, as I said in this case you are better off this way:
main()
{
  Wind w;
  Tower t;

  startYourRadarThread(&w, &t);
  startYourTowerThread(&w, &t);
}
Avatar of John500

ASKER

The 'main' below shows the whole program from a glance.  That is --> the user can view Wind, Radar, and Aircraft info from socket connections transmitted over a real network.  The application may need to provide a drop-down menu item that causes it to enter into an "Air Traffic Control" mode.  From a design standpoint I don't know if this is necessary.

I would like to imagine that when the user chooses to view information it is always being processed in the background to accomidate a landing.  Moreover, if the user only chose to view Wind, how would that impact the Tower thread and queue ?  I would think if the Wind socket was the only socket running, the Tower would have nothing in the queue.

If the Aircraft thread was started (which collects socket info on aircraft desiring to land)  then this would constitute grounds for establishing a queue.  This may justifiably be the Tower thread.  When it is started it does the following:

1)  collects aircraft info on aircraft desiring to land
2)  creates a queue to hold the structures that will have all necessary info (Aircraft, Wind, Radar)
3)  contains the logic to handle the queue :  
          look to the head to see if it has all information for decision making
          make the decision and shuffle the queue as necessary

The other threads (Wind, Radar) will do the following:

1)  Check to see if access to the queue is possible (has it been created)
2)  If the queue actually exists, only update the head of the queue

.... period, end of story.   That's the way I see it to accomplish the goals of :

1)  casual observation of socket data
2)  not-so-casual observation --->  displaying landing decisions

Alex, does anything change in terms of your ideas for design ?

int APIENTRY _tWinMain(HINSTANCE hInstance,
                     HINSTANCE hPrevInstance,
                     LPTSTR    lpCmdLine,
                     int       nCmdShow)
{

      
      System::Threading::Thread::CurrentThread->ApartmentState = System::Threading::ApartmentState::STA;
      
      // Establish socket connections to necessary dependent dlls/libs.
      //  These connections primarily handle parsing packets for the
      //  Wind, Radar & Aircraft sockets as they are being read
      GetSocketHandles();

      // Start the application
      Application::Run(new Form1());

      //////////////////////////////////////////////////////////////////////////////
      //   From this point on the user chooses the ‘View’ drop-down      
      //   menu to view Radar, Wind, Aircraft  or all three.  Each kind    
      //   of data is obtained by socket connections.   Each can be shut  
      //   off by returning to the ‘View’ menu and removing the check  
      //   mark.                                                                                          
      ///////////////////////////////////////////////////////////////////////////////

      // terminate all sockets if somehow they are still open
      WSACleanup();

      // Application ended - free socket libraries
      if (ParseLib) FreeLibrary(ParseLib);
      if (PackLib) FreeLibrary(PackLib);
      if (SockLib) FreeLibrary(SockLib);
      
      return 0;
}
dbkruger, you are absolutely right. 'new' and 'delete' *have* to be in the same module.

However, we have a problem here what might justify an exception of that rule. Thread creation and main already were done by the .NET framework, which is based on the wizard-generated Form1 class here. I don't think Form1 class should be more than a framework and a GUI. So thread-creation can *not* be used to pass the objects (it already passes a pointer to the Form1 object) *and* we have some objects that were created dynamically. Hence, not Form1 but (one of) the threads have to create the objects. We could take Tower to create all other objects (and delete them in its destructor). Form1 could hold a member of Tower and would delete the Tower singleton (and all subsequent objects) in its destructor. Fine. However, all other threads would need to get Tower first (no problem, I already described that in the code above) *and* use Tower member functions to create new objects, e. g. Aircraft, Wind, Radar. That solves the problem of 'new' and 'delete' but isn't a good OO model. Why should Tower create Aircraft objects that came from a simulation implemented in the Aircraft thread? Or, if we say Tower creates only proxies we would need the *originals* in the other threads, what makes the whole thing unnecessarily complex. To have both - the threads create their objects and delete them in the thread proc (or thread class) - we would need to pass pointers of these objects to Tower where they got stored in appropriate members. Then, Tower has to clear these members prior to them being deleted in the threads or at end of simulation we would crash. Clearing can be done  by additional notifications and some efforts of synchronisation when the simulation is going to be terminated. But, I think deletion of objects and safe thread-termination can be considered later.

Regards, Alex
 
>>>> Alex, does anything change in terms of your ideas for design ?

I don't know if I understand all *your* ideas till now ;-)

These sockets were very suspect? Who is sending the information you were talking about?

Generally, my design would be as follows:

1. All threads were running an infinite loop that was exited by request from the GUI only.
2. When closing the form, class Form1 sets a bool member that was checked by all threads in the loop.
3. The Form1 waits for all threads having recognized termination
4. Any thread - beside of Tower - creates its objects (randomly or by request) and passes them to Tower
5. Then, these objects periodically were updated in a thread-safe way.
6. After each update, Tower was notificated.
7. The form was updated as well, so the user can see what is happening.
7. Any single Aircraft handles notifications of the Tower regarding permission of landing.
8. For that, Tower updates parts of the Aircraft record.
9. Tower has exclusive access to the Landing queue and shows its current status in the form
10. Tower periodically checks notifications of all other objects and updates the landing queue.
11. Based on the current information of the landing queue it was shuffled what might lead to
      updates and corresponding notifications to the Aircraft objects.
12. The user may send requests to either thread (via socket ???), thus take influence to the simulation.
13. These requests need to be handled by the threads and passed to their objects.

Hope, it was understandable.

Regards, Alex

     
Avatar of John500

ASKER

>> Hope, it was understandable

Yes it was, thank you.

Now that you've had some time, does my method make sense ?

>> 4. Any thread - beside of Tower - creates its objects (randomly or by request) and passes them to Tower
I would contend this is not necessary.  Why - because the functional aspect of this application is the driver.

>> Who is sending the information you were talking about?

These transmissions are from models and/or simulators for the purpose of R&D.

At this point, my tasking is two fold:

1)  Ramp up to established network protocol and the associated socket routines
2)  Provide a tool (GUI) for validation/viewing of network traffic (packet collection)

Eventually - hook up models and simulators - tweak accordinly

At this point part  '1' is done and part '2' is as good as done.  Why - from the standpoint that all required data is viewable with the simple flip of a drop-down menu checkmark.  If it's on, user sees data;  if its off, user doesn't see data.

I'm done.  See you lator have a nice day.  But (there's always the 'but') ...  soon the next task will center around a simulator that assigns ' line-up for landing '   **an order strictly determined outside this application **.

May I say *strictly* again ?  lol    Just kiddin with ya

... Since the application is this far along and functional, I want to incorporate a method (I've been advocating) that uses the aircraft thread as the driver to this application.  At any time I can take the aircraft data that was transmitted and make a decision --->  Since it's R&D and M&S, it's like creating your own decision execution theory - Its my panel kind-of-thing - where simple is better.  I just dont' know how to set-up the mutex for this particular approach.

Believe me, your suggestions (Alex/dbkruger) are much appreciated.  I shall do my very best at awarding appropriately if swayed one way or the other.  Right now, I want to compile code and if it's yours Alex - let's do this.  I still say my way is doable !!

Aircraft thread:
------------------
1)  collects aircraft info on aircraft desiring to land
2)  creates a queue to hold the structures that will have all necessary info (Aircraft, Wind, Radar)
3)  contains the logic to handle the queue :  
          look to the head to see if it has all information for decision making
          make the decision and shuffle the queue as necessary

Wind, Radar thread :
------------------------
1)  Check to see if access to the queue is possible (has it been created)
2)  If the queue actually exists, only update the head of the queue

Actually there are other threads but I've kept this simple to avoid unnecessary confusion as to 'my' scope
>>>> Now that you've had some time, does my method make sense ?

Depending on what you mean by "my method"  ;-)

If you mean the implementation of _tWinMain, it seems all ok to me.

If you mean that Aircraft thread should run the landing queue, I still think that is a wrong OO model. Maybe it's simply a naming problem. I fully agree to the tasks 1,2,3 if you woukd replace the title "Aircraft thread" by "Tower thread".

1)  collects aircraft info on aircraft desiring to land
2)  creates a queue to hold the structures that will have all necessary info (Aircraft, Wind, Radar)
3)  contains the logic to handle the queue :  
          look to the head to see if it has all information for decision making
          make the decision and shuffle the queue as necessary


The tasks of Aircraft thread were

1) Creates new arriving Aircrafts randomly or by request from GUI and passes them to Tower.
2) Periodically updates all Aircraft data such as distance from runway, direction, height, waiting status
3) Handles notifications sent to any single Aircraft from Tower, e. g. permission to land
4) Updates screen data regarding the aircrafts

Wind, Radar thread :

1) Create Wind and Radar objects.
2) Pass the objects to the Tower
3) Periodically update wind/radar information
4) Handle user requests
5) Update screen data


You might start to implement. We will help when you are stuck.

Regards, Alex
Avatar of John500

ASKER

Alex,

I added a new header file from the code you posted here:

Comment from itsmeandnobodyelse
Date: 05/03/2006 01:05PM EST

Therefore, I now have:

NotificationCode functionality
class AirportObject
class Notification

The code compiled 'as is' without any modification whatsoever, not one #include<>, nothing !  I guess that's all good.....   The questions I have are these:

1)  Is it correct to have this new header file (I'm calling AirportObject) in a separate header file or should I use this code to make four similar header files?  You wrote :

>>  You now should derive Tower, Aircraft, Wind, Radar from AirportObject.

What does that mean?  Are you saying I should create four new classes which have the just the same notification enumeration?  What else would they have?  I need help here - help me create one of these classes (Tower, AircraftInfo, Wind, Radar) and I can duplicate the rest (maybe)

2)  You wrote:

>> I would suggest to add a (public) member pointer of the Tower object to the Form1 class

I'm still not clear what a tower object is comprised of.  Maybe that class would be best to start with

After you answer this question, I'll 'accept' and start another question which will hopefully be the last.

Thanks !
ASKER CERTIFIED SOLUTION
Avatar of itsmeandnobodyelse
itsmeandnobodyelse
Flag of Germany image

Link to home
membership
This solution is only available to members.
To access this solution, you must be a member of Experts Exchange.
Start Free Trial