Opened 13 years ago

Closed 13 years ago

Last modified 13 years ago

#3381 closed patch (fixed)

Fix memory leak and other minor pthread usage issues in FIFOWriter

Reported by: anonymous Owned by: Stuart Auchterlonie
Priority: minor Milestone: 0.21
Component: mythtv Version: head
Severity: medium Keywords:
Cc: Ticket locked: no

Description

This patch addresses a few issues that I noticed while browsing through libs/libmythtv/fifowriter.cpp:

  • The array of locks, fifo_lock, was not deleted in the FIFOWriter destructor.
  • pthread_mutex_init() was not called on the array of locks in the FIFOWriter constructor. This is usually not that big of a deal on Linux because the default initial value of pthread_mutex_t is zeroed out. However, it can lead to weird problems on other platforms.
  • pthread_mutex_destroy() and pthread_cond_destroy() were not called on the two arrays of thread conditions and the one array of locks in the FIFOWriter destructor. This can potentially create memory leaks.
  • There are two places where the empty_cond condition is signaled without locking the appropriate mutex.

IRC: russellb

Attachments (3)

fifowriter.patch (1.9 KB) - added by russell@… 13 years ago.
fifowriter.rev2.patch (12.2 KB) - added by russell@… 13 years ago.
fifowriter.rev3.patch (1.8 KB) - added by russell@… 13 years ago.

Download all attachments as: .zip

Change History (9)

Changed 13 years ago by russell@…

Attachment: fifowriter.patch added

comment:1 Changed 13 years ago by Stuart Auchterlonie

Owner: changed from Isaac Richards to Stuart Auchterlonie

Initial testing of this and i've not managed to lockup my frontend by causing it to generate too many previews.

Would other people with issues with the preview thread please test this and report back.

Stuart

Changed 13 years ago by russell@…

Attachment: fifowriter.rev2.patch added

comment:2 Changed 13 years ago by russell@…

I have attached a second patch: fifowriter.rev2.patch, which addresses the original concerns as well as the following:

  • Converts the locks and thread conditions in FIFOWriter from using the pthread API directly, to using QMutex and QWaitCondition.
  • Converts the locks and thread conditions in HDTVRecorder to the same.

comment:3 Changed 13 years ago by russell@…

Note that my point about pthread_mutex_init() not getting called in FIFOWriter wasn't actually correct. However, the rest is still true. :)

Changed 13 years ago by russell@…

Attachment: fifowriter.rev3.patch added

comment:4 Changed 13 years ago by anonymous

Even though stuarta said I didn't have to, I posted another patch that *only* contains the bug fixes, and no "code cleanup" changes. I will submit those changes separately. Also, the patch was created from the root of the project instead of a subdirectory.

comment:5 Changed 13 years ago by Stuart Auchterlonie

Resolution: fixed
Status: newclosed

(In [13443]) Closes #3381. Applies patch from russellb that cleans up the lock handling and thread usage in fifowriter.

Been running this for a while and it makes things more stable.

comment:6 Changed 13 years ago by Stuart Auchterlonie

Milestone: unknown0.21
Note: See TracTickets for help on using tickets.