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 2 years ago

Closed 22 months ago

#10311 closed Developer Task (fixed)

Cleanup reference counting

Reported by: danielk Owned by: danielk
Priority: minor Milestone: 0.26
Component: MythTV - General Version: Master Head
Severity: medium Keywords:
Cc: Ticket locked: no

Description

There are some reference counters in MythTV, in particular in MythUI that start at 0 not 1.

This can cause a problem mostly because programmers assume a conventional reference counter API.

We can also improve the performance of our reference counters by using QAtomicInteger.

After we fix the API, we must rename the reference counter functions so that 3rd party users of the API won't be caught out by the change without warning.

Attachments (0)

Change History (16)

comment:1 Changed 2 years ago by wagnerrp

  • Status changed from new to assigned
  • Version changed from Unspecified to Master Head

comment:2 Changed 2 years ago by stuartm

  • Type changed from Bug Report - General to Developer Task

comment:3 Changed 2 years ago by Daniel Kristjansson <danielk@…>

In 517a34867563021abd084490dd367e78e98d3b60/mythtv:

Refs #10311. Reimplements RefererenceCounter? using QAtomicInt for lockless reference counting.

This also renames AddRef? to IncrRef? and DownRef? to DecrRef?. This is solely for the benefit of any plugins that might be using the reference counters in MythUI when this is extended to those objects. The MythUI reference counters start at zero instead of one so anything using those will need to remove the UpRef/AddRef? immediately post creation.

comment:4 Changed 23 months ago by Daniel Thor Kristjansson <danielk@…>

comment:5 Changed 23 months ago by Daniel Thor Kristjansson <danielk@…>

In f0bfe0f0737e918536b0dd8aca466c3d37df6032/mythtv:

Refs #10311. Add additional debugging capabilities to ReferenceCounter?.

The original ReferenceCounter? class would print out the QObject's objectName().
I didn't implement this when I rewrote ReferenceCounter? as a general purpose
reference counter. But having a name for an object does aid in debugging, this
reintroduces that idea but with a QString passed into the constructor. By
default this is not enabled, but if you need to debug reference counting
enabling it is just a matter of uncommenting a "#define EXTRA_DEBUG" in
the header and recompiling.

comment:6 Changed 23 months ago by Daniel Thor Kristjansson <danielk@…>

In 62d42998fb878d31262ac19a78b003bbf7a0e9b3/mythtv:

Refs #10311. Make reference counter more extensible.

comment:7 Changed 23 months ago by Daniel Thor Kristjansson <danielk@…>

comment:8 Changed 23 months ago by Daniel Thor Kristjansson <danielk@…>

comment:9 Changed 23 months ago by Daniel Thor Kristjansson <danielk@…>

comment:10 Changed 23 months ago by Daniel Kristjansson <danielk@…>

In 7e98bdb1b9f05dd42ac6076557afe696ded79666/mythtv:

Refs #10311. Use reference counting for MythRenderVDPAU.

Stuart Morgan noticed an error message on Friday that pointed to
non-use of the reference counter in a reference counted class.
This makes the respective classes use reference counting and
makes the MythRenderVDPAU destructor private so any such use will
be caught at compile time in the future.

comment:11 Changed 23 months ago by Daniel Kristjansson <danielk@…>

comment:12 Changed 23 months ago by Daniel Kristjansson <danielk@…>

comment:13 Changed 22 months ago by Daniel Kristjansson <danielk@…>

In 263eef17762223b9e47b67c032330b88490963bd/mythtv:

Refs #10311. Add leak debugging to the ReferenceCounter?. Disabled by default.

comment:14 Changed 22 months ago by Daniel Kristjansson <danielk@…>

In 1b8f1f32c92b167e50863a73545207f80cc7c328/mythtv:

Refs #10311. Use R/W lock for the leak detection.

Also exclude CommandLineArg? from the leak report, these go away after the ReferenceCounter::PrintDebug?() is called.
Also add extra debugging for MythImage? classes, but only on linux since it can adds a cross lib dependency with OSX doesn't like.
Also use static cast for MythImage? to avoid linking problems.

comment:15 Changed 22 months ago by Daniel Kristjansson <danielk@…>

In 6b04160ac3ad1666cd034aa809ed5bb939a2028c/mythtv:

Refs #10311. Port MythUI to ReferenceCounter?.

Note: MythUIButtonList::GetImage?() has been renamed to GetImageFilename?(),
this was done to avoid conflict as getImage was renamed GetImage?() and
because it is more descriptive.

As far as I can tell all MythUI leaks have now been plugged.

comment:16 Changed 22 months ago by Daniel Thor Kristjansson <danielk@…>

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

In 0433370c01313ba010ca3d9d7757bd14e8f63d69/mythtv:

Fixes #10311. Convert VAAPI context to use ReferenceCounter?.

Note: I am unable to test this here, but I've read over it a few times to
try to account for all the ways it could fail.

This is the last of the UpRef/DownRef? -> IncrRef/DecrRef? changes.

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.