Modify
Warning Please read the Ticket HowTo before creating or commenting on a ticket. Failure to do so may cause your ticket to be rejected or result in a slower response.

Opened 16 months ago

Closed 10 months ago

Last modified 10 months ago

#11341 closed Bug Report - General (fixed)

RemoteFile::Open() can leak MythSockets

Reported by: paulh <mythtv@…> Owned by: cpinkham
Priority: minor Milestone: 0.27
Component: MythTV - General Version: Master Head
Severity: medium Keywords:
Cc: Ticket locked: no

Description

RemoteFile::Open(void) doesn't check to see if it's sockets have already been opened so it's possible for some new sockets to be created without first closing and deleting the old ones.

Things are made worse by the fact that the RemoteFile? constructor will automatically call Open() if a url is passed to it.

These are a few places where a RemoteFile? is created with a url passed to it and later Open() is called :- There is one in MusicSGIODevice which is where I first spotted the problem while playing back tracks from the Music storage group. There is a couple in MetadataDownload? readMXML() and readNFO().

These all cause the Error: Not all threads were shut down properly: Thread MythSocketThread?(-1) is still running warnings when exiting the FE.

Attachments (0)

Change History (4)

comment:1 Changed 15 months ago by cpinkham

  • Owner set to cpinkham
  • Status changed from new to assigned

comment:2 Changed 10 months ago by Jim Stichnoth <jstichnoth@…>

In ba75244a7f7e412bdb100b7741eaa6a6f0e6649b/mythtv:

Use RemoteFile::isOpen() instead of Open() to check for success.

Calling Open() was unnecessarily creating extra socket connections to
the backend, and the original socket connections from calling Open()
in the constructor were leaking and accumulating in both the frontend
and backend (refs #11341). Refs #11618

comment:3 Changed 10 months ago by Chris Pinkham <cpinkham@…>

  • Resolution set to fixed
  • Status changed from assigned to closed

In 4c9dc6d7eebe6853285efbe76a2f81ad80ebbc3c/mythtv:

RemoteFile? leak fix and simplification

RemoteFile::Open() would leak file descriptors if Open() was explicitly
called again after passing a url to the contructor which itself called
Open().

This commit does several things:

  • Moves Open() private since RemoteFile? users shouldn't need to Open()
  • Moves Close() private since there's no need to Close() if you can't Open(). Close() is already called in the destructor.
  • Convert the remaining uses of Open() to isOpen() since the connection is already opened in all these cases.
  • Remove a few Close() uses since they were were redundant since we already Close() in the destructor.
  • Removes unused SetURL() functionality. If you can't Open() or Close(), then you need to pass the URL in to the constructor so there's no need.
  • Fixes the actual leak in Open() by checking to see if the sockets are already open. Just in case.... Closes #11341.

NOTE: This does modify the binary ABI version number due to the remotefile.h

changes, so make clean, etc..

comment:4 Changed 10 months ago by cpinkham

  • Milestone changed from unknown to 0.27

Add Comment

Modify Ticket

Action
as closed .
The resolution will be deleted. Next status will be 'new'.
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.