Radek Polak pson...@seznam.cz writes:
On Thursday, November 08, 2012 09:12:40 AM Neil Jerram wrote:
With current QtMoko git I'm seeing a lot of QWSLock errors in my log,
[...]
To me it looked that the semaphore was closed by the other process or thread.
The logging and some other stuff was added there in Qt 4.8.
I believe I've understood and fixed this - please see the attached
patch. I've been running with this change for a week and a bit now,
with no apparent bad consequences, so I guess it's OK to go into the
main repository.
Regards,
Neil
From 52c34001bad85c3032618070b1d6b2a3c6880715 Mon Sep 17 00:00:00 2001
From: Neil Jerram n...@ossau.homelinux.net
Date: Thu, 8 Nov 2012 08:18:32 +
Subject: [PATCH] Fix QWSLock invalid argument logs
There was no known actual problem associated with these logs, but they
were spamming the log, so I thought it worth trying to understand and
fix them.
The confusion is that there are two different ways of creating QWSLock
objects. QWSLock() creates an object that creates a new set of
semaphores, whereas QWSLock(id) creates an object that aliases the
existing set of semaphores with ID id. What seems to happen is that
each application creates a semaphore set scoped to that
application (QWSDisplay::Data::clientLock in qapplication_qws.cpp),
then this semaphore set is passed by complex means to
places (QWSClientPrivate and QWSMemorySurface) that use the semaphores
for a short period and then delete their QWSLock objects.
The problem was that the QWSLock destructor always destroyed the
semaphore set, even when that QWSLock hadn't create the semaphores
itself, hence making the semaphores invalid for other QWSLock objects
still referencing the same set.
Clearly a QWSLock object shouldn't destroy the semaphore set if it
didn't create it itself, and that is confirmed by the fact that one of
the implementations inside QWSLock already implements this logic, with
the 'owned' flag. The fix is to implement this for the #ifndef
QT_POSIX_IPC case - which is what is used in QtMoko - just as is
already implemented for the #ifdef QT_POSIX_IPC case.
---
src/gui/embedded/qwindowsystem_qws.cpp |3 +++
src/gui/embedded/qwslock.cpp| 17 +
src/gui/embedded/qwslock_p.h|2 +-
src/gui/kernel/qapplication_qws.cpp |2 ++
src/gui/painting/qwindowsurface_qws.cpp |2 ++
5 files changed, 21 insertions(+), 5 deletions(-)
diff --git a/src/gui/embedded/qwindowsystem_qws.cpp b/src/gui/embedded/qwindowsystem_qws.cpp
index a1c613d..1ed8b6d 100644
--- a/src/gui/embedded/qwindowsystem_qws.cpp
+++ b/src/gui/embedded/qwindowsystem_qws.cpp
@@ -680,6 +680,7 @@ QWSClientPrivate::QWSClientPrivate()
QWSClientPrivate::~QWSClientPrivate()
{
#ifndef QT_NO_QWS_MULTIPROCESS
+//qDebug(QWSClientPrivate::~QWSClientPrivate());
delete clientLock;
#endif
}
@@ -689,7 +690,9 @@ void QWSClientPrivate::setLockId(int id)
#ifdef QT_NO_QWS_MULTIPROCESS
Q_UNUSED(id);
#else
+//qDebug(QWSClientPrivate::setLockId(%d), id);
clientLock = new QWSLock(id);
+//qDebug(= %p, clientLock);
#endif
}
diff --git a/src/gui/embedded/qwslock.cpp b/src/gui/embedded/qwslock.cpp
index 9914a24..1055785 100644
--- a/src/gui/embedded/qwslock.cpp
+++ b/src/gui/embedded/qwslock.cpp
@@ -83,9 +83,13 @@ QWSLock::QWSLock(int id) : semId(id)
QWSSignalHandler::instance()-addWSLock(this);
#endif
+owned = false;
+
#ifndef QT_POSIX_IPC
if (semId == -1) {
semId = semget(IPC_PRIVATE, 3, IPC_CREAT | 0666);
+owned = true;
+ //qDebug(QWSLock::QWSLock(): %p, %d, this, (int)semId);
if (semId == -1) {
perror(QWSLock::QWSLock);
qFatal(Unable to create semaphore);
@@ -100,7 +104,6 @@ QWSLock::QWSLock(int id) : semId(id)
}
#else
sems[0] = sems[1] = sems[2] = SEM_FAILED;
-owned = false;
if (semId == -1) {
// ### generate really unique IDs
@@ -134,9 +137,12 @@ QWSLock::~QWSLock()
if (semId != -1) {
#ifndef QT_POSIX_IPC
-qt_semun semval;
-semval.val = 0;
-semctl(semId, 0, IPC_RMID, semval);
+ if (owned) {
+ qt_semun semval;
+ semval.val = 0;
+ semctl(semId, 0, IPC_RMID, semval);
+ }
+ //qDebug(QWSLock::~QWSLock(): %p, %d, this, (int)semId);
semId = -1;
#else
// emulate the SEM_UNDO behavior for the BackingStore lock
@@ -170,8 +176,10 @@ bool QWSLock::up(unsigned short semNum)
if (semNum == BackingStore)
sops.sem_flg |= SEM_UNDO;
+//qDebug(QWSLock::up(): %p, semop(%d, %d), this, (int)semId, (int)semNum);
EINTR_LOOP(ret, semop(semId, sops, 1));
#else
+//qDebug(QWSLock::up(): %p, sem_post(%d), this, (int)(sems[semNum]));
ret = sem_post(sems[semNum]);
#endif
if (ret == -1) {
@@ -195,6 +203,7 @@ bool QWSLock::down(unsigned short semNum, int)
if (semNum == BackingStore)
sops.sem_flg |= SEM_UNDO;
+//qDebug(QWSLock::down(): %p, semop(%d, %d),
