Patrick Williams | c124f4f | 2015-09-15 14:41:29 -0500 | [diff] [blame] | 1 | From 52c34001bad85c3032618070b1d6b2a3c6880715 Mon Sep 17 00:00:00 2001 |
| 2 | From: Neil Jerram <n...@ossau.homelinux.net> |
| 3 | Date: Thu, 8 Nov 2012 08:18:32 +0000 |
| 4 | Subject: [PATCH] Fix QWSLock "invalid argument" logs |
| 5 | |
| 6 | There was no known actual problem associated with these logs, but they |
| 7 | were spamming the log, so I thought it worth trying to understand and |
| 8 | fix them. |
| 9 | |
| 10 | The confusion is that there are two different ways of creating QWSLock |
| 11 | objects. "QWSLock()" creates an object that creates a new set of |
| 12 | semaphores, whereas "QWSLock(id)" creates an object that aliases the |
| 13 | existing set of semaphores with ID id. What seems to happen is that |
| 14 | each application creates a semaphore set scoped to that |
| 15 | application (QWSDisplay::Data::clientLock in qapplication_qws.cpp), |
| 16 | then this semaphore set is passed by complex means to |
| 17 | places (QWSClientPrivate and QWSMemorySurface) that use the semaphores |
| 18 | for a short period and then delete their QWSLock objects. |
| 19 | |
| 20 | The problem was that the QWSLock destructor always destroyed the |
| 21 | semaphore set, even when that QWSLock hadn't create the semaphores |
| 22 | itself, hence making the semaphores invalid for other QWSLock objects |
| 23 | still referencing the same set. |
| 24 | |
| 25 | Clearly a QWSLock object shouldn't destroy the semaphore set if it |
| 26 | didn't create it itself, and that is confirmed by the fact that one of |
| 27 | the implementations inside QWSLock already implements this logic, with |
| 28 | the 'owned' flag. The fix is to implement this for the #ifndef |
| 29 | QT_POSIX_IPC case - which is what is used in QtMoko - just as is |
| 30 | already implemented for the #ifdef QT_POSIX_IPC case. |
| 31 | |
| 32 | Original patch can be found here: |
| 33 | http://www.mail-archive.com/community@lists.openmoko.org/msg65512.html |
| 34 | |
| 35 | Upstream-Status: Submitted |
| 36 | |
| 37 | Signed-off-by: Mike Looijmans <mike.looijmans@topic.nl> |
| 38 | (Removed the commented-out debug statements from the original patch.) |
| 39 | |
| 40 | --- |
| 41 | |
| 42 | diff --git a/src/gui/embedded/qwslock.cpp b/src/gui/embedded/qwslock.cpp |
| 43 | index 9914a24..1055785 100644 |
| 44 | --- a/src/gui/embedded/qwslock.cpp |
| 45 | +++ b/src/gui/embedded/qwslock.cpp |
| 46 | @@ -83,9 +83,12 @@ QWSLock::QWSLock(int id) : semId(id) |
| 47 | QWSSignalHandler::instance()->addWSLock(this); |
| 48 | #endif |
| 49 | |
| 50 | + owned = false; |
| 51 | + |
| 52 | #ifndef QT_POSIX_IPC |
| 53 | if (semId == -1) { |
| 54 | semId = semget(IPC_PRIVATE, 3, IPC_CREAT | 0666); |
| 55 | + owned = true; |
| 56 | if (semId == -1) { |
| 57 | perror("QWSLock::QWSLock"); |
| 58 | qFatal("Unable to create semaphore"); |
| 59 | @@ -100,7 +104,6 @@ QWSLock::QWSLock(int id) : semId(id) |
| 60 | } |
| 61 | #else |
| 62 | sems[0] = sems[1] = sems[2] = SEM_FAILED; |
| 63 | - owned = false; |
| 64 | |
| 65 | if (semId == -1) { |
| 66 | // ### generate really unique IDs |
| 67 | @@ -134,9 +137,11 @@ QWSLock::~QWSLock() |
| 68 | |
| 69 | if (semId != -1) { |
| 70 | #ifndef QT_POSIX_IPC |
| 71 | - qt_semun semval; |
| 72 | - semval.val = 0; |
| 73 | - semctl(semId, 0, IPC_RMID, semval); |
| 74 | + if (owned) { |
| 75 | + qt_semun semval; |
| 76 | + semval.val = 0; |
| 77 | + semctl(semId, 0, IPC_RMID, semval); |
| 78 | + } |
| 79 | semId = -1; |
| 80 | #else |
| 81 | // emulate the SEM_UNDO behavior for the BackingStore lock |
| 82 | diff --git a/src/gui/embedded/qwslock_p.h b/src/gui/embedded/qwslock_p.h |
| 83 | index d324e4f..d867d20 100644 |
| 84 | --- a/src/gui/embedded/qwslock_p.h |
| 85 | +++ b/src/gui/embedded/qwslock_p.h |
| 86 | @@ -86,8 +86,8 @@ private: |
| 87 | int lockCount[2]; |
| 88 | #ifdef QT_POSIX_IPC |
| 89 | sem_t *sems[3]; |
| 90 | - bool owned; |
| 91 | #endif |
| 92 | + bool owned; |
| 93 | }; |
| 94 | |
| 95 | QT_END_NAMESPACE |
| 96 | |
| 97 | -- |
| 98 | 1.7.10.4 |