[AGENT++] Why need the Agentpp::Synchronized class a pthread_cond_t cont member?

[Frank Fock]

Hi Claus,

What problem/issue are you trying to fix?
Are you trying to improve performance or stability?
If the latter, did you find any race condition in the existing code?

The Synchronized class was there with a simple mutex before the requirement to implement a lock queue was raised.
Its current design is thus mainly influenced by the its initial purpose.  

The main purpose of the LockQueue is to being able to unlock an object by a different thread than it was locked by.
I do not see how this can be achieved by the quoted library. 
I general I do not want to unnecessary dependencies to third party libraries because that could create unwanted interference with libraries the users of the AGENT++ API are using.

Best regards,
Frank

> On 16. Nov 2018, at 23:09, Claus Klein <claus.klein at arcormail.de> wrote:
> 
> Hi all
> 
> As far as I know, each MIB object is a Synchronized class.
> 
> And the only reason for the pthread_cond_t at Synchronized Class apart from ThreadPool seems this LockRequest class:
> 
> void RequestList::lock_request(Request* req)
> {
> #ifdef NO_FAST_MUTEXES
>    if (lockQueue) {
>        LockRequest r(req);
>        lockQueue->acquire(&r);
>        r.wait();
>    }
> #else
>    req->lock();
> #endif
> }
> 
> void RequestList::unlock_request(Request* req)
> {
> #ifdef NO_FAST_MUTEXES
>    if (lockQueue) {
>        LockRequest r(req);
>        lockQueue->release(&r);
>        r.wait();
>    }
> #else
>    req->unlock();
> #endif
> }
> 
> I know locking more than one mutex is not simple. But there is a generic lock algorithm:
> see https://en.cppreference.com/w/cpp/thread/lock
> and https://en.cppreference.com/w/cpp/named_req/Lockable
> 
> Note:
> Boost provides a version of this function that takes a sequence of „Lockable" objects defined by a pair of iterators.
> std::scoped_lock offers a RAII wrapper for this function, and is generally preferred to a naked call to std::lock.
> see https://www.boost.org/doc/libs/1_68_0/doc/html/thread/synchronization.html#thread.synchronization.lock_functions.lock_range
> 
> 
> At the current code, the wait() is waiting for as signal, but does not know the condition the application is waiting for!
> Too it is not asserted, that the mutex is locket, but this is required!
> 
> void Synchronized::wait()
> {
>    cond_timed_wait(0);
> }
> 
> int Synchronized::cond_timed_wait(const struct timespec* ts)
> {
>    int result;
>    isLocked = FALSE;
>    if (ts) {
>        result = pthread_cond_timedwait(&cond, &monitor, ts);
>    } else {
>        result = pthread_cond_wait(&cond, &monitor);
>    }
>    isLocked = TRUE;
>    return result;
> }
> 
> // ATTENTION: this code is not exception save! CK
> void ThreadPool::execute(Runnable* t)
> {
>    lock();
>    //=======
>    TaskManager* tm = 0;
>    while (!tm) {
>        ArrayCursor<TaskManager> cur;
>        for (cur.init(&taskList); cur.get(); cur.next()) {
>            tm = cur.get();
>            if (tm->is_idle()) {
>                // TaskManager: task manager found
>                //=======
>                unlock();
>                if (tm->set_task(t)) {
>                    return;
>                } else {
>                    tm = 0; // task could not be assigned
>                }
>                lock();
>                //=======
>            }
>            tm = 0;
>        }
>        if (!tm) {
>            wait(1000);
>        }
>    }
>    //=======
>    unlock();
> }
> 
> Notes
> std::condition_variable_any can be used with std::shared_lock in order to wait on a std::shared_mutex in shared ownership mode.
> A possible use for std::condition_variable_any with custom „Lockable“ types is to provide convenient interruptible waits: the custom lock operation would both lock the associated mutex as expected, and also perform the necessary setup to notify this condition variable when the interrupting signal is received.
> 
> References
> 	• ↑ A. Williams (2012), "C++ concurrency in action" 9.2.4 Interrupting a wait on std::condition_variable_any
> 
> 
> see too https://en.cppreference.com/w/cpp/thread/condition_variable
> and https://en.cppreference.com/w/cpp/thread/condition_variable_any/wait
> 
> 
> =================
> So this is my question:
> 
> Would it not a better design to implement at Synchronized class only a „Lockable“ concept?
> Than it could be used with std::lock<> or boost::lock<> and too in code like this:
> 
> std::condition_variable_any cv;
> std::mutex cv_m; // This mutex is used for three purposes:
>                 // 1) to synchronize accesses to i
>                 // 2) to synchronize accesses to std::cerr
>                 // 3) for the condition variable cv
> int i = 0; 	// the shared variable to signal some state
> 
> void waits_until_i_is_1()
> {
>    std::unique_lock<std::mutex> lk(cv_m);
>    std::cerr << "Waiting... \n";
>    cv.wait(lk, [] { return i == 1; });
>    std::cerr << „...finished waiting. i == 1\n";
> }
> 
> 
> Regards
> Claus
> 
> 
> 
> 
> 
> 
> _______________________________________________
> AGENTPP mailing list
> AGENTPP at agentpp.org
> https://oosnmp.net/mailman/listinfo/agentpp