Designfrage: Pool



  • Hallo!
    ich benötige für meinen Server Thread- u. Socketpools und habe deshalb beschlossen, eine Klasse dafür zu schreiben. Später sollen dann verschiedene Threads auf den Pool zugreifen, ein Element des Pools benutzen und wenn sie fertig sind wieder zurücklegen. Das ist mein grober Entwurf:

    template <typename HTy_>
    class pool {
    public:
    	typedef typename std::list<HTy_>::iterator iterator;
    
    	template <typename Fnc_>
    	pool(unsigned poolsize, const Fnc_& fnc) {
    		while(--poolsize) unused_.push(fnc());
    	}
    
    	const iterator get() {
    		used_.push_front(unused_.front());
    		unused_.pop();
    		return used.begin();
    	}
    
    	void putback(iterator) {
    		unused_.push(*iterator);
    		used_.erase(iterator);
    	}
    
    private:
    	std::queue<HTy_> unused_;
    	std::list<HTy_> used_;
    };
    

    Da die gespeicherten Handles ja entweder benutzt o. gerade nicht benutzt werden, hatte ich die Idee sie jeweils zwischen den Containern hin und her zu moven. Aber schon die Wahl der Container fiel mir hier schwer und ich bin mir nicht sicher, ob ich mir damit noch irgendwann einen Strick gedreht habe.
    Also, gibt es bei diesem Entwurf irgendwo eine Falle, die mir später Kopfschmerzen bereiten kann?



  • Da der Pool von mehreren Threads benutzt wird, muss der Zugriff synchronisiert werden. Je nachdem wie oft darauf zugegriffen wird, bzw. wie lange eine Resource benutzt wird, kann das mehr oder minder den parallelen Ablauf stoeren.



  • > muss der Zugriff synchronisiert werden.

    Was genau heißt das? Ich hatte vor, dass jeder Thread einen Zeiger auf den selben Speicherbereich (der, wo der Pool liegt) bekommt. Dann müsste der Pool doch bei jedem Thread synchron, also gleich sein?



  • Das sind Grundlagen, die du dir selbst aneignen musst. Hier ein Link zum Starten: http://de.wikipedia.org/wiki/Race_Condition



  • Hm... okay, ich habe das jetzt so umgeändert:

    template <typename HTy_>
    class pool {
    public:
    	typedef typename std::list<HTy_>::iterator iterator;
    
    	template <typename Fnc_>
    	pool(unsigned poolsize, const Fnc_& fnc)
    	{
    		while(--poolsize) unused_.push(fnc());
    	}
    
    	const iterator get() {
    		mutex lock;
    		used_.push_front(unused_.front());
    		unused_.pop();
    		return used.begin();
    	}
    
    	void putback(iterator) {
    		mutex lock;
    		unused_.push(*iterator);
    		used_.erase(iterator);
    	}
    
    private:
    	std::queue<HTy_> unused_;
    	std::list<HTy_> used_;
    };
    

    Hoffentlich geht das gut... 😞



  • was soll "mutex lock;" sein?



  • Na so ein Lock-Teil, das ich anderswo definiert habe. Wäre aber Windows-spezifisch.



  • .. habe meine Antwort versucht klarer zu schreiben.. 🙂

    Edit:
    Andersrum: Du musst pro zu lockenden Bereich (also einen Bereich auf welchen nur 1 Thread gleichzeitig zugreifen kann) ein Mutex Objekt haben. Bedeutet, dein Objekt lock muss ein Member sein!

    Simon



  • Deshalb besitzt mutex ja im Innern ein statisches Objekt. 😕 Der Konstruktor is blockierend, fals dieses Objekt im "locked"-mode steht.



  • Aha.. aber Du willst ja nicht alle Pool Instanzen auch gleich mitlocken, wenn Du bei einer bestimmten Pool Instanz ein Objekt rausholst.

    Edit:
    Typeischerweise geht das in etwa so:
    1. Es gibt ein Mutex Objekt (CriticalSection, Mutex, etc.) pro zu schützenden "Bereich", in deinem Fall ein Objekt vom Typ pool.
    2. Jede Methode die den schützenswerten Bereich "anfassen" möchte, versucht dieses Mutex Objekt zu locken und wird es danach auch wieder freigeben. Das wird oft mittels RAII, z.B. einer Lock Klasse gemacht, welche als Konstruktor Argument eben dieses Mutex Objekt nimmt.



  • So habe ich es in etwa gemacht:

    template <typename HTy_>
    class pool {
    public:
        typedef typename std::list<HTy_>::iterator iterator;
    
        template <typename Fnc_>
        pool(unsigned poolsize, const Fnc_& fnc)
        {
            while(--poolsize) unused_.push(fnc());
        }
    
        const iterator get() {
            mutex lck(lock_);
            used_.push_front(unused_.front());
            unused_.pop();
            return used.begin();
        }
    
        void putback(iterator) {
            mutex lck(lock_);
            unused_.push(*iterator);
            used_.erase(iterator);
        }
    
    private:
        mutex lock_;
        std::queue<HTy_> unused_;
        std::list<HTy_> used_;
    };
    

    Jetzt in Ordnung?



  • Ohne genau zu wissen was dein "mutex" ist, kann das keiner sagen, aber das was hier nach nem Copyctor aussieht ist dann wohl falsch.



  • > Ohne genau zu wissen was dein "mutex" ist,

    Da nicht alle Leute Win32-API können, bringt es nichts, den Code zu posten.

    > aber das was hier nach nem Copyctor aussieht ist dann wohl falsch.

    Nö. Der Lock wird kopiert und blockiert dann, wenn gerade jemand anderes dort arbeitet. Wenn der andere fertig ist, hört der Copy-Ctor auf zu warten und blockiert nun selber und returned. Der Dtor von der Kopie hebt die Blockade dann wieder auf.



  • Ad aCTa schrieb:

    > Ohne genau zu wissen was dein "mutex" ist,

    Da nicht alle Leute Win32-API können, bringt es nichts, den Code zu posten.

    dann können wir auch nix machen



  • Mir egal, aber für dieses Forum ist das jetzt nicht treffend:

    class mutex {
    public:
    	mutex()
               : event_(CreateEvent(0, 0, 0, 0)) {
    		SetEvent(event_); // signaled
    	}
    
    	mutex(const mutex& m)
    		: event_(m.event_) {
    		WaitForSingleObject(event_, INFINITE);
    		ResetEvent(event_); // Nicht signaled
    	}
    
    	~mutex() { SetEvent(event_); } // signaled
    
    private:
    	HANDLE event_;
    };
    


  • Ad aCTa schrieb:

    Jetzt in Ordnung?

    Nein, du darfst keinen iterator auf den pool nach aussen geben.
    Davon abgesehen hast du hier ja keinen relevanten Code, also kann man nichts sagen. Aber der Ansatz sieht etwas schief aus. Vorallem der mutex Code und der Ctor.

    Weiters musst du mit Proxys der Resourcen arbeiten, du willst ja nicht dauernd kopieren - vorallem sind nicht alle Resourcen kopierbar...

    Die used Liste brauchst du im Prinzip auch nur wenn du garbage collection machen willst.

    Vielleicht waere etwas mehr Code schon sinnvoll, denn da ist nichts worueber wir sinnvoll reden koennten, mit Ausnahme ob man eine used-Liste braucht.



  • > Nein, du darfst keinen iterator auf den pool nach aussen geben.

    Warum?

    > Davon abgesehen hast du hier ja keinen relevanten Code, also kann man nichts sagen.

    Anwendungsfall wäre:

    typedef std::shared_ptr<socket> socket_ptr;
    pool<socket_ptr> socket_pool(5, create_socket); // create_socket ist eine Funktion
    // ...
    for(;;) {
       pool<socket_ptr>::iterator accept_socket = socket_pool.get(); // freien Socket bekommen
       accept(*accept_socket); // Verbindung aufnehmen
       service(*accpet_socket); // Verbindung bearbeiten bis zum disconnect
       socket_pool.putback(accept_socket); // Socket zurücklegen
    }
    

    Zugegeben, das Dereferenzieren sieht nicht schön aus, aber wo ist das Problem mit einem Iterator?

    > Weiters musst du mit Proxys der Resourcen arbeiten, du willst ja nicht dauernd kopieren

    Ich habe sowohl Wrapper um die Handles alsauch shared_ptr.



  • was soll dein mutex jetzt machen außer ewig warten im copyctor?
    schau dir mal das an
    http://msdn.microsoft.com/en-us/library/ms686927(VS.85).aspx
    und so geht ein scoped lock
    http://www.boost.org/doc/libs/1_35_0/doc/html/boost/interprocess/scoped_lock.html

    Ad aCTa schrieb:

    > Nein, du darfst keinen iterator auf den pool nach aussen geben.

    Warum?

    weil man damit alles möglich ein deinem pool machen kann, am mutex vorbei usw..



  • > was soll dein mutex jetzt machen außer ewig warten im copyctor?

    Das Event umschalten (non-signaled), damit ein weiterer Lock jetzt warten muss. Davor wartet er solange, bis ein anderer Thread das Event auf signaled stellt. (Genau dann wenn der Dtor eines anderen Locks aufgerufen wurde.) Was soll daran nicht stimmen?

    > weil man damit alles möglich ein deinem pool machen kann, am mutex vorbei usw..

    Was will man denn machen? Der Iterator ist konstant, der Pool kann durch ihn nicht verändert werden. Er hat nur Zugriff auf dieses eine Element, und das gehört jetzt dem Thread und muss bis zum putback nicht mehr synchronisiert werden.

    Edit: Ja, der Sinn der used-Liste ist schon etwas fraglich. Ich hatte Angst vor Leaks, weil ich dem Nutzer nun das Handle anvertraue und hoffe, dass er es auch wieder in den Pool zurückpackt. Ich könnte das natürlich so machen:

    struct noch_ein_wrapper {
       noch_ein_wrapper(const socket_ptr& ptr, pool<socket_ptr>& p)
          : socket_pool(p), socket(ptr) {
       }
    
       ~noch_ein_wrapper() { socket_pool.putback(ptr); }
    
       pool<socket_ptr>& socket_pool;
       socket_ptr socket;
    };
    

    Aber das wäre jetzt schon der 3. Wrapper um das eigentliche Handle...



  • 1. CloseHandle(..) wird nicht aufgerufen bei deiner mutex Implemenation.
    2. Benutzt doch die extra dafür vorgesehenen Mechanismen, entweder boost oder direkt Win32 API Critical Section, Mutex, Semaphore, ...

    Hier eine Implementation mit Critical Sections um das Prinzip zu Skizieren:

    class CriticalSection
    {
    public:
        CriticalSection()
        {
          InitializeCriticalSection(&_cs);
        }
        ~CriticalSection()
        {
          DeleteCriticalSection(&_cs);
        }
        void enter() const
        {
          EnterCriticalSection(&_cs);
        }
        void leave() const
        {
          LeaveCriticalSection(&_cs);
        }
    
    private:
        CriticalSection(const CriticalSection&);
        CriticalSection& operator=(const CriticalSection&);
    
        mutable CRITICAL_SECTION _cs;
    };
    
    class CriticalSectionLock
    {
    public:
        explicit CriticalSectionLock(const CriticalSection& cs)
        : _cs(cs)
        {
          _cs.enter();
        }
        ~CriticalSectionLock()
        {
          _cs.leave();
        }
    
    private:
        CriticalSectionLock(const CriticalSectionLock&);
        CriticalSectionLock& operator=(const CriticalSectionLock&);
    
        const CriticalSection& _cs;
    };
    

    Simon

    PS:

    Da nicht alle Leute Win32-API können, bringt es nichts, den Code zu posten.

    Glaub mir, da wissen ne Menge Leute ganz anderes nicht und es würde nie etwas bringen irgendetwas zu posten.. aber ist ja offensichtlich nicht so...


Anmelden zum Antworten