thermald: does not check lock file correctly

Bug #1480892 reported by Colin Ian King
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
thermald (Ubuntu)
Fix Released
High
Colin Ian King
Trusty
Fix Released
High
Colin Ian King
Vivid
Fix Released
High
Colin Ian King
Wily
Fix Released
High
Colin Ian King

Bug Description

Commit 97b1720a42f33fd21e6fbf30b6b3857a948e5633 ("Fix Coverity error in release") closes the lockfile and hence we can now run multiple instances of thermald.

The regression was caused by overzealous file descriptor leakage cleanups to stop static analysis warnings. The locking should be improved by:

1. Making the fd global
2. Closing and unlinking the lock file on abort exits or normal terminations

Changed in thermald (Ubuntu):
status: New → In Progress
importance: Undecided → Medium
assignee: nobody → Colin Ian King (colin-king)
importance: Medium → High
Revision history for this message
Colin Ian King (colin-king) wrote :

[SRU Justification] (Vivid, Trusty)

Thermald lockfile has been broken during a code cleanup, meaning multiple instances of thermald can be started. This fix is required to re-instate the lockfile functionality while also performing correct closure and removal of the lock file on thermald termination (be it an error or a successful termination). Thus this fix restores the original functionality plus it also cleans up the lockfile too.

[Test Case]
Without the fix, one can start multiple instances of thermald, e.g run the following while thermald is running as a service:

sudo thermald --no-daemon --loglevel=info

With the fix, thermald will detect another instance of thermald (via the lockfile) and terminate with an error message, for example:

Couldn't get lock file 21449
An instance of thermald is already running, exiting ...

[Regression Potential]
Can't think of any.

Changed in thermald (Ubuntu Trusty):
status: New → In Progress
Changed in thermald (Ubuntu Vivid):
status: New → In Progress
Changed in thermald (Ubuntu Trusty):
importance: Undecided → High
Changed in thermald (Ubuntu Vivid):
importance: Undecided → High
Changed in thermald (Ubuntu Trusty):
assignee: nobody → Colin Ian King (colin-king)
Changed in thermald (Ubuntu Vivid):
assignee: nobody → Colin Ian King (colin-king)
Revision history for this message
Chris J Arges (arges) wrote : Please test proposed package

Hello Colin, or anyone else affected,

Accepted thermald into trusty-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/thermald/1.4.3-3~14.04.1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-needed to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

Changed in thermald (Ubuntu Trusty):
status: In Progress → Fix Committed
tags: added: verification-needed
Revision history for this message
Chris J Arges (arges) wrote :

Hello Colin, or anyone else affected,

Accepted thermald into vivid-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/thermald/1.4.3-3~15.04.1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-needed to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

Changed in thermald (Ubuntu Vivid):
status: In Progress → Fix Committed
Revision history for this message
Colin Ian King (colin-king) wrote :

Verified on trusty -proposed, without fix, can start up multiple instances, with the fix, only one instance can be started, locking is fixed as expected.

Revision history for this message
Colin Ian King (colin-king) wrote :

Verified on vivid -proposed, without fix, can start up multiple instances, with the fix, only one instance can be started, locking is fixed as expected.

tags: added: trusty verification-done vivid
removed: verification-needed
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package thermald - 1.4.3-3~14.04.1

---------------
thermald (1.4.3-3~14.04.1) trusty; urgency=medium

  * Fix null pointer deferences with _ART active trip points (LP: #1480821)
  * Re-enable lockfile (LP: #1480892)
  * Report multiple instances of thermald with thd_log_fatal (LP: #1432220)
  * Suppress systemd failed service messages (e.g. on VMs) (LP: #1480182)

 -- Colin King <email address hidden> Tue, 4 Aug 2015 11:50:00 +0100

Changed in thermald (Ubuntu Trusty):
status: Fix Committed → Fix Released
Revision history for this message
Adam Conrad (adconrad) wrote : Update Released

The verification of the Stable Release Update for thermald has completed successfully and the package has now been released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.

Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package thermald - 1.4.3-3~15.04.1

---------------
thermald (1.4.3-3~15.04.1) vivid; urgency=medium

  * Fix null pointer deferences with _ART active trip points (LP: #1480821)
  * Re-enable lockfile (LP: #1480892)
  * Report multiple instances of thermald with thd_log_fatal (LP: #1432220)
  * Suppress systemd failed service messages (e.g. on VMs) (LP: #1480182)

 -- Colin King <email address hidden> Tue, 4 Aug 2015 11:48:00 +0100

Changed in thermald (Ubuntu Vivid):
status: Fix Committed → Fix Released
Changed in thermald (Ubuntu Wily):
status: In Progress → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers