Centralized 'thread active' handling
Commit Message
In order to fix a possible race condition in cTransfer I have
centralized the 'active' handling of cThread. Since this is a
rather sensitive area of the program, I'd like to invite you
all to take a look at the attached patch against VDR version
1.3.28 and let me know if you're running into any trouble
with it.
Here's the HISTORY entry I made for this change:
Fixed a race condition in cTransfer (thanks to Klaus ??? for reporting this one).
In doing so, the 'active' variables used by the actual derived cThread classes
have been replaced by the cThread::Active() function. The previous functionality
of cThread::Active() has been moved into the new cThread::Running().
Plugin authors should check their derived cThread classes and replace any 'active'
variables the same way as, for instance, done in transfer.c.
Klaus
Comments
On 13 Aug 2005 Klaus Schmidinger <Klaus.Schmidinger@cadsoft.de> wrote:
Hi,
> Fixed a race condition in cTransfer (thanks to Klaus ??? for reporting this one).
> In doing so, the 'active' variables used by the actual derived cThread classes
> have been replaced by the cThread::Active() function. The previous functionality
> of cThread::Active() has been moved into the new cThread::Running().
> Plugin authors should check their derived cThread classes and replace any 'active'
> variables the same way as, for instance, done in transfer.c.
Wouldn't it be better to leave Active() untouched (instead of
renaming to Running()) and create a new (differently named)
function for the active var replacement? (e.g. Continue(), which
would give good readability with while(Continue()))
This would limit the impact this change has to plugins.
Otherwise a plugins using Active() now, could compile fine with
the next VDR version, but certainly wouldn't work correctly.
Regards.
Stefan Huelswitt wrote:
> Wouldn't it be better to leave Active() untouched (instead of
> renaming to Running()) and create a new (differently named)
> function for the active var replacement? (e.g. Continue(), which
> would give good readability with while(Continue()))
In that case I'd suggest Terminated()
while(!Terminated())
it would somewhat reduce my confusion when switching from the elegance
of delphi/lazarus to the awkwardness of C++ ;-)
Bye
Stefan Huelswitt wrote:
> On 13 Aug 2005 Klaus Schmidinger <Klaus.Schmidinger@cadsoft.de> wrote:
>
> Hi,
>
>
>>Fixed a race condition in cTransfer (thanks to Klaus ??? for reporting this one).
>>In doing so, the 'active' variables used by the actual derived cThread classes
>>have been replaced by the cThread::Active() function. The previous functionality
>>of cThread::Active() has been moved into the new cThread::Running().
>>Plugin authors should check their derived cThread classes and replace any 'active'
>>variables the same way as, for instance, done in transfer.c.
>
>
> Wouldn't it be better to leave Active() untouched (instead of
> renaming to Running()) and create a new (differently named)
> function for the active var replacement? (e.g. Continue(), which
> would give good readability with while(Continue()))
Well, Active() reflects the internal 'active' variable, so
Continue() would have to correspond to 'continue', which is
a reserved keyword...
> This would limit the impact this change has to plugins.
>
> Otherwise a plugins using Active() now, could compile fine with
> the next VDR version, but certainly wouldn't work correctly.
Is there any plugin that actually uses cThread::Active()?
Klaus
Luca Olivetti wrote:
> Stefan Huelswitt wrote:
>
>> Wouldn't it be better to leave Active() untouched (instead of
>> renaming to Running()) and create a new (differently named)
>> function for the active var replacement? (e.g. Continue(), which
>> would give good readability with while(Continue()))
>
>
> In that case I'd suggest Terminated()
>
> while(!Terminated())
>
> it would somewhat reduce my confusion when switching from the elegance
> of delphi/lazarus to the awkwardness of C++ ;-)
Why use an extra negation here?
I think a positive check ('Active()') is more straightforward
than a negative one ('!Terminated()').
Just wondering: what does this have to do with "elegance" vs. "awkwardness"?
Klaus
On 13 Aug 2005 Klaus Schmidinger <Klaus.Schmidinger@cadsoft.de> wrote:
> Stefan Huelswitt wrote:
>>
>> Otherwise a plugins using Active() now, could compile fine with
>> the next VDR version, but certainly wouldn't work correctly.
>
> Is there any plugin that actually uses cThread::Active()?
Both mp3 & mplayer plugin do.
To make it worse, they don't access the old "active" var, so I
guess your changes won't make them fail compile.
Regards.
On 13 Aug 2005 Klaus Schmidinger <Klaus.Schmidinger@cadsoft.de> wrote:
> Stefan Huelswitt wrote:
>>
>> Wouldn't it be better to leave Active() untouched (instead of
>> renaming to Running()) and create a new (differently named)
>> function for the active var replacement? (e.g. Continue(), which
>> would give good readability with while(Continue()))
>
> Well, Active() reflects the internal 'active' variable, so
> Continue() would have to correspond to 'continue', which is
> a reserved keyword...
This is after your change, but before Active() didn't reflected
any internal variable. I don't know if one must build such a
strict rule ...
Just my two cents.
Regards.
Stefan Huelswitt wrote:
> On 13 Aug 2005 Klaus Schmidinger <Klaus.Schmidinger@cadsoft.de> wrote:
>
>>Stefan Huelswitt wrote:
>>
>>>Otherwise a plugins using Active() now, could compile fine with
>>>the next VDR version, but certainly wouldn't work correctly.
>>
>>Is there any plugin that actually uses cThread::Active()?
>
>
> Both mp3 & mplayer plugin do.
And are you sure that this is _really_ necessary?
Do they _really_ want to determine whether the thread
is still running, or would they just like to know whether
it's time to return from the Action() function?
I don't know the internals of these plugins, therefore
my question.
> To make it worse, they don't access the old "active" var, so I
> guess your changes won't make them fail compile.
That variable was private to some derived cThread classes in VDR,
so they couldn't have accessed them, anyway. I was referring to
cases where plugins might have used an 'active' variable (or whatever
they may have called it) in the same way VDR did.
Klaus
Stefan Huelswitt wrote:
> On 13 Aug 2005 Klaus Schmidinger <Klaus.Schmidinger@cadsoft.de> wrote:
>
>>Stefan Huelswitt wrote:
>>
>>>Wouldn't it be better to leave Active() untouched (instead of
>>>renaming to Running()) and create a new (differently named)
>>>function for the active var replacement? (e.g. Continue(), which
>>>would give good readability with while(Continue()))
>>
>>Well, Active() reflects the internal 'active' variable, so
>>Continue() would have to correspond to 'continue', which is
>>a reserved keyword...
>
>
> This is after your change, but before Active() didn't reflected
> any internal variable. I don't know if one must build such a
> strict rule ...
Of course you don't have to, but it's nice ;-)
Klaus
Klaus Schmidinger wrote:
>> it would somewhat reduce my confusion when switching from the elegance
>> of delphi/lazarus to the awkwardness of C++ ;-)
>
>
> Why use an extra negation here?
> I think a positive check ('Active()') is more straightforward
> than a negative one ('!Terminated()').
It mostly depends on habit. The native delphi class for threads (and the
corresponding one in freepascal), TThread, just uses Terminate instead
of Cancel, and the corresponding property is Terminated, so I'm just
used to wrap a thread in a "while not terminated".
> Just wondering: what does this have to do with "elegance" vs.
> "awkwardness"?
Nothing, it was a tongue-in-cheek remark.
Bye
Klaus Schmidinger wrote:
> Stefan Huelswitt wrote:
>
>> On 13 Aug 2005 Klaus Schmidinger <Klaus.Schmidinger@cadsoft.de> wrote:
>>
>> Hi,
>>
>>
>>> Fixed a race condition in cTransfer (thanks to Klaus ??? for
>>> reporting this one).
>>> In doing so, the 'active' variables used by the actual derived
>>> cThread classes
>>> have been replaced by the cThread::Active() function. The previous
>>> functionality
>>> of cThread::Active() has been moved into the new cThread::Running().
>>> Plugin authors should check their derived cThread classes and
>>> replace any 'active'
>>> variables the same way as, for instance, done in transfer.c.
>>
>>
>>
>> Wouldn't it be better to leave Active() untouched (instead of
>> renaming to Running()) and create a new (differently named)
>> function for the active var replacement? (e.g. Continue(), which
>> would give good readability with while(Continue()))
>
>
> Well, Active() reflects the internal 'active' variable, so
> Continue() would have to correspond to 'continue', which is
> a reserved keyword...
>
What has the name of a method to do with te name of a internal variable?
>> This would limit the impact this change has to plugins.
>>
>> Otherwise a plugins using Active() now, could compile fine with
>> the next VDR version, but certainly wouldn't work correctly.
>
>
> Is there any plugin that actually uses cThread::Active()?
/In the vbox plugin i use some code from mp3 plugin and there i have a
Active() call which simply returns active member of cTrhead.
IMO what stfean huelswitt says , is right.
regards,
gunnar
/
Gunnar Roth wrote:
> Klaus Schmidinger wrote:
>
>> Stefan Huelswitt wrote:
>>
>>> On 13 Aug 2005 Klaus Schmidinger <Klaus.Schmidinger@cadsoft.de> wrote:
>>>
>>> Hi,
>>>
>>>
>>>> Fixed a race condition in cTransfer (thanks to Klaus ??? for
>>>> reporting this one).
>>>> In doing so, the 'active' variables used by the actual derived
>>>> cThread classes
>>>> have been replaced by the cThread::Active() function. The previous
>>>> functionality
>>>> of cThread::Active() has been moved into the new cThread::Running().
>>>> Plugin authors should check their derived cThread classes and
>>>> replace any 'active'
>>>> variables the same way as, for instance, done in transfer.c.
>>>
>>>
>>>
>>>
>>> Wouldn't it be better to leave Active() untouched (instead of
>>> renaming to Running()) and create a new (differently named)
>>> function for the active var replacement? (e.g. Continue(), which
>>> would give good readability with while(Continue()))
>>
>>
>>
>> Well, Active() reflects the internal 'active' variable, so
>> Continue() would have to correspond to 'continue', which is
>> a reserved keyword...
>>
> What has the name of a method to do with te name of a internal variable?
Well, nothing, technically - it's just that I like to do it that way.
Now, before you start dissecting VDR and showing me places where I did
it differently, let's just end this here ;-)
>>> This would limit the impact this change has to plugins.
>>>
>>> Otherwise a plugins using Active() now, could compile fine with
>>> the next VDR version, but certainly wouldn't work correctly.
>>
>>
>>
>> Is there any plugin that actually uses cThread::Active()?
>
>
> /In the vbox plugin i use some code from mp3 plugin and there i have a
> Active() call which simply returns active member of cTrhead.
>
> IMO what stfean huelswitt says , is right.
Ok, I give up.
I'll leave the old Active() function as it was and call the new one
something like Continue() or so. I won't call it Terminate() or anyting
that requires an extra negation, though.
Now, with the naming problem settled, are there any actual, technical
issues with this modification?
Klaus
>
> Now, with the naming problem settled, are there any actual, technical
> issues with this modification?
>
A quick compile test (using the patch in this thread) and all my plugins build
except for autotimeredit-0.1.6:
g++ -g -O2 -Wall -Woverloaded-virtual -c -DPLUGIN_NAME_I18N='"autotimeredit"'
-D_GNU_SOURCE -I../../../include -I/usr/src/DVB/linux/include autotimeredit.c
autotimeredit.c: In member function `virtual void cPluginAutoTimer::Housekeeping()':
../../../include/vdr/thread.h:94: error: `bool cThread::Active()' is protected
autotimeredit.c:948: error: within this context
make[1]: *** [autotimeredit.o] Error 1
Regards.
On 13 Aug 2005 Klaus Schmidinger <Klaus.Schmidinger@cadsoft.de> wrote:
> Stefan Huelswitt wrote:
>> On 13 Aug 2005 Klaus Schmidinger <Klaus.Schmidinger@cadsoft.de> wrote:
>>
>>>Stefan Huelswitt wrote:
>>>
>>>>Otherwise a plugins using Active() now, could compile fine with
>>>>the next VDR version, but certainly wouldn't work correctly.
>>>
>>>Is there any plugin that actually uses cThread::Active()?
>>
>>
>> Both mp3 & mplayer plugin do.
>
> And are you sure that this is _really_ necessary?
> Do they _really_ want to determine whether the thread
> is still running, or would they just like to know whether
> it's time to return from the Action() function?
> I don't know the internals of these plugins, therefore
> my question.
I dunno if it's necessary, but for me it's common practice in a
player plugin. The derived cControl class calls player->Active()
so see if the derived cPlayer class (which in most cases is a
cThread as well) is still running. Like in:
eOSState cMP3Control::ProcessKey(eKeys Key)
{
if(!player->Active()) return osEnd;
[...]
But there are other cases where the call is usefull.
And yes, if Active() is called, I want to know if the thread is
still running, as this is the only purpose of this function (up
to now).
You may argue, that I'm using (the old) Active() in an incorrect
way, but nevertheless I think that any API change should be made
in a way that existing plugins either continue to work properly
or fail to compile. Anything else will lead to massive confusion
for the average user.
>> To make it worse, they don't access the old "active" var, so I
>> guess your changes won't make them fail compile.
>
> That variable was private to some derived cThread classes in VDR,
> so they couldn't have accessed them, anyway. I was referring to
> cases where plugins might have used an 'active' variable (or whatever
> they may have called it) in the same way VDR did.
You're right. My plugins use an own variable but in a similiar
way as the internal one.
Regards.
On 13 Aug 2005 Klaus Schmidinger <Klaus.Schmidinger@cadsoft.de> wrote:
> Now, with the naming problem settled, are there any actual, technical
> issues with this modification?
*g*
If you could provide an updated patch, I'll go and test it.
Regards.
Stefan Huelswitt wrote:
>
> You're right. My plugins use an own variable but in a similiar
> way as the internal one.
In fact if this is done right it will avoid a lot of duplicated code in
derived classes.
Bye
Stefan Huelswitt wrote:
> On 13 Aug 2005 Klaus Schmidinger <Klaus.Schmidinger@cadsoft.de> wrote:
>
>
>>Now, with the naming problem settled, are there any actual, technical
>>issues with this modification?
>
>
> *g*
>
> If you could provide an updated patch, I'll go and test it.
I'll provide one tomorrow.
Have to think about the name some more.
With Continue() we get some funny things like
if (!Continue())
Start();
while
if (!Active())
Start();
would have made sense ;-)
Klaus
Klaus Schmidinger wrote:
> Stefan Huelswitt wrote:
>
>> On 13 Aug 2005 Klaus Schmidinger <Klaus.Schmidinger@cadsoft.de> wrote:
>>
>>
>>> Now, with the naming problem settled, are there any actual, technical
>>> issues with this modification?
>>
>>
>>
>> *g*
>>
>> If you could provide an updated patch, I'll go and test it.
>
>
> I'll provide one tomorrow.
> Have to think about the name some more.
I dont seem to have any trouble with the first one (except that one patch I had
to make for the autotimeredit plugin).
Best Regards,
On Sat, 2005-08-13 at 18:36 +0200, Klaus Schmidinger wrote:
> Have to think about the name some more.
Alive() ?
Klaus Schmidinger wrote:
> I'll provide one tomorrow.
> Have to think about the name some more.
> With Continue() we get some funny things like
>
> if (!Continue())
> Start();
>
> while
>
> if (!Active())
> Start();
>
> would have made sense ;-)
if (Terminated())
Start();
also does.
Catch you ;-)
Anyway, Looping(), Running() (since it wasn't used before), Activated()
(though easily confused with Active()), Execute()......
Bye
===================================================================
RCS file: ./RCS/cutter.c
retrieving revision 1.8
@@ -18,7 +18,6 @@
class cCuttingThread : public cThread {
private:
const char *error;
- bool active;
int fromFile, toFile;
cFileName *fromFileName, *toFileName;
cIndexFile *fromIndex, *toIndex;
@@ -35,7 +34,6 @@
:cThread("video cutting")
{
error = NULL;
- active = false;
fromFile = toFile = -1;
fromFileName = toFileName = NULL;
fromIndex = toIndex = NULL;
@@ -53,7 +51,6 @@
cCuttingThread::~cCuttingThread()
{
- active = false;
Cancel(3);
delete fromFileName;
delete toFileName;
@@ -67,7 +64,8 @@
if (Mark) {
fromFile = fromFileName->Open();
toFile = toFileName->Open();
- active = fromFile >= 0 && toFile >= 0;
+ if (fromFile < 0 || toFile < 0)
+ return;
int Index = Mark->position;
Mark = fromMarks.Next(Mark);
int FileSize = 0;
@@ -78,7 +76,7 @@
uchar buffer[MAXFRAMESIZE];
bool LastMark = false;
bool cutIn = true;
- while (active) {
+ while (Active()) {
uchar FileNumber;
int FileOffset, Length;
uchar PictureType;
@@ -215,7 +213,7 @@
void cCutter::Stop(void)
{
- bool Interrupted = cuttingThread && cuttingThread->Active();
+ bool Interrupted = cuttingThread && cuttingThread->Running();
const char *Error = cuttingThread ? cuttingThread->Error() : NULL;
delete cuttingThread;
cuttingThread = NULL;
@@ -232,7 +230,7 @@
bool cCutter::Active(void)
{
if (cuttingThread) {
- if (cuttingThread->Active())
+ if (cuttingThread->Running())
return true;
error = cuttingThread->Error();
Stop();
===================================================================
RCS file: ./RCS/cutter.h
retrieving revision 1.1
===================================================================
RCS file: ./RCS/device.c
retrieving revision 1.103
@@ -156,8 +156,6 @@
SetVideoFormat(Setup.VideoFormat);
- active = false;
-
mute = false;
volume = Setup.CurrentVolume;
@@ -1126,25 +1124,25 @@
void cDevice::Action(void)
{
- if (active && OpenDvr()) {
- for (; active;) {
- // Read data from the DVR device:
- uchar *b = NULL;
- if (GetTSPacket(b)) {
- if (b) {
- int Pid = (((uint16_t)b[1] & PID_MASK_HI) << 8) | b[2];
- // Distribute the packet to all attached receivers:
- Lock();
- for (int i = 0; i < MAXRECEIVERS; i++) {
- if (receiver[i] && receiver[i]->WantsPid(Pid))
- receiver[i]->Receive(b, TS_SIZE);
- }
- Unlock();
- }
- }
- else
- break;
- }
+ if (Active() && OpenDvr()) {
+ while (Active()) {
+ // Read data from the DVR device:
+ uchar *b = NULL;
+ if (GetTSPacket(b)) {
+ if (b) {
+ int Pid = (((uint16_t)b[1] & PID_MASK_HI) << 8) | b[2];
+ // Distribute the packet to all attached receivers:
+ Lock();
+ for (int i = 0; i < MAXRECEIVERS; i++) {
+ if (receiver[i] && receiver[i]->WantsPid(Pid))
+ receiver[i]->Receive(b, TS_SIZE);
+ }
+ Unlock();
+ }
+ }
+ else
+ break;
+ }
CloseDvr();
}
}
@@ -1188,10 +1186,8 @@
Receiver->device = this;
receiver[i] = Receiver;
Unlock();
- if (!active) {
- active = true;
+ if (!Active())
Start();
- }
return true;
}
}
@@ -1218,10 +1214,8 @@
else if (receiver[i])
receiversLeft = true;
}
- if (!receiversLeft) {
- active = false;
+ if (!receiversLeft)
Cancel(3);
- }
}
void cDevice::DetachAll(int Pid)
@@ -1246,13 +1240,11 @@
delivered = false;
ringBuffer = new cRingBufferLinear(Size, TS_SIZE, true, "TS");
ringBuffer->SetTimeouts(100, 100);
- active = true;
Start();
}
cTSBuffer::~cTSBuffer()
{
- active = false;
Cancel(3);
delete ringBuffer;
}
@@ -1262,20 +1254,20 @@
if (ringBuffer) {
bool firstRead = true;
cPoller Poller(f);
- for (; active;) {
- if (firstRead || Poller.Poll(100)) {
- firstRead = false;
- int r = ringBuffer->Read(f);
- if (r < 0 && FATALERRNO) {
- if (errno == EOVERFLOW)
- esyslog("ERROR: driver buffer overflow on device %d", cardIndex);
- else {
- LOG_ERROR;
- break;
- }
- }
- }
- }
+ while (Active()) {
+ if (firstRead || Poller.Poll(100)) {
+ firstRead = false;
+ int r = ringBuffer->Read(f);
+ if (r < 0 && FATALERRNO) {
+ if (errno == EOVERFLOW)
+ esyslog("ERROR: driver buffer overflow on device %d", cardIndex);
+ else {
+ LOG_ERROR;
+ break;
+ }
+ }
+ }
+ }
}
}
===================================================================
RCS file: ./RCS/device.h
retrieving revision 1.60
@@ -233,7 +233,6 @@
// PID handle facilities
private:
- bool active;
virtual void Action(void);
protected:
enum ePidType { ptAudio, ptVideo, ptPcr, ptTeletext, ptDolby, ptOther };
@@ -518,7 +517,6 @@
private:
int f;
int cardIndex;
- bool active;
bool delivered;
cRingBufferLinear *ringBuffer;
virtual void Action(void);
===================================================================
RCS file: ./RCS/dvbdevice.c
retrieving revision 1.131
@@ -76,7 +76,6 @@
cCiHandler *ciHandler;
cChannel channel;
const char *diseqcCommands;
- bool active;
bool useCa;
time_t startTime;
eTunerStatus tunerStatus;
@@ -101,7 +100,6 @@
frontendType = FrontendType;
ciHandler = CiHandler;
diseqcCommands = NULL;
- active = false;
useCa = false;
tunerStatus = tsIdle;
startTime = time(NULL);
@@ -113,7 +111,6 @@
cDvbTuner::~cDvbTuner()
{
- active = false;
tunerStatus = tsIdle;
newSet.Signal();
Cancel(3);
@@ -294,8 +291,7 @@
void cDvbTuner::Action(void)
{
dvb_frontend_event event;
- active = true;
- while (active) {
+ while (Active()) {
Lock();
if (tunerStatus == tsSet) {
while (GetFrontendEvent(event))
===================================================================
RCS file: ./RCS/dvbplayer.c
retrieving revision 1.36
@@ -79,7 +79,6 @@
int wanted;
int length;
bool hasData;
- bool active;
cCondWait newSet;
protected:
void Action(void);
@@ -98,13 +97,11 @@
buffer = NULL;
wanted = length = 0;
hasData = false;
- active = false;
Start();
}
cNonBlockingFileReader::~cNonBlockingFileReader()
{
- active = false;
newSet.Signal();
Cancel(3);
free(buffer);
@@ -147,8 +144,7 @@
void cNonBlockingFileReader::Action(void)
{
- active = true;
- while (active) {
+ while (Active()) {
Lock();
if (!hasData && f >= 0 && buffer) {
int r = safe_read(f, buffer + length, wanted - length);
@@ -187,8 +183,6 @@
cIndexFile *index;
int replayFile;
bool eof;
- bool active;
- bool running;
bool firstPacket;
ePlayModes playMode;
ePlayDirs playDir;
@@ -207,7 +201,7 @@
public:
cDvbPlayer(const char *FileName);
virtual ~cDvbPlayer();
- bool Active(void) { return active; }
+ bool Active(void) { return cThread::Active(); }
void Pause(void);
void Play(void);
void Forward(void);
@@ -233,8 +227,6 @@
backTrace = NULL;
index = NULL;
eof = false;
- active = true;
- running = false;
firstPacket = true;
playMode = pmPlay;
playDir = pdForward;
@@ -353,11 +345,8 @@
if (replayFile >= 0)
Start();
}
- else if (active) {
- running = false;
+ else
Cancel(9);
- active = false;
- }
}
void cDvbPlayer::Action(void)
@@ -374,8 +363,7 @@
int Length = 0;
bool Sleep = false;
- running = true;
- while (running && (NextFile() || readIndex >= 0 || ringBuffer->Available() || !DeviceFlush(100))) {
+ while (Active() && (NextFile() || readIndex >= 0 || ringBuffer->Available() || !DeviceFlush(100))) {
if (Sleep) {
cCondWait::SleepMs(3); // this keeps the CPU load low
Sleep = false;
@@ -501,7 +489,6 @@
Sleep = true;
}
}
- active = running = false;
cNonBlockingFileReader *nbfr = nonBlockingFileReader;
nonBlockingFileReader = NULL;
===================================================================
RCS file: ./RCS/dvbplayer.h
retrieving revision 1.2
===================================================================
RCS file: ./RCS/recorder.c
retrieving revision 1.13
@@ -29,7 +29,6 @@
uchar pictureType;
int fileSize;
int recordFile;
- bool active;
time_t lastDiskSpaceCheck;
bool RunningLowOnDiskSpace(void);
bool NextFile(void);
@@ -43,7 +42,6 @@
cFileWriter::cFileWriter(const char *FileName, cRemux *Remux)
:cThread("file writer")
{
- active = false;
fileName = NULL;
remux = Remux;
index = NULL;
@@ -63,7 +61,6 @@
cFileWriter::~cFileWriter()
{
- active = false;
Cancel(3);
delete index;
delete fileName;
@@ -96,13 +93,11 @@
void cFileWriter::Action(void)
{
time_t t = time(NULL);
- active = true;
- while (active) {
+ while (Active()) {
int Count;
uchar *p = remux->Get(Count, &pictureType);
if (p) {
- //XXX+ active??? see old version (Busy)
- if (!active && pictureType == I_FRAME) // finish the recording before the next 'I' frame
+ if (!Active() && pictureType == I_FRAME) // finish the recording before the next 'I' frame
break;
if (NextFile()) {
if (index && pictureType != NO_PICTURE)
@@ -124,15 +119,12 @@
t = time(NULL);
}
}
- active = false;
}
cRecorder::cRecorder(const char *FileName, int Ca, int Priority, int VPid, const int *APids, const int *DPids, const int *SPids)
:cReceiver(Ca, Priority, VPid, APids, Setup.UseDolbyDigital ? DPids : NULL, SPids)
,cThread("recording")
{
- active = false;
-
// Make sure the disk is up and running:
SpinUpDisk(FileName);
@@ -157,25 +149,22 @@
writer->Start();
Start();
}
- else if (active) {
- active = false;
+ else
Cancel(3);
- }
}
void cRecorder::Receive(uchar *Data, int Length)
{
- if (active) {
+ if (Active()) {
int p = ringBuffer->Put(Data, Length);
- if (p != Length && active)
+ if (p != Length && Active())
ringBuffer->ReportOverflow(Length - p);
}
}
void cRecorder::Action(void)
{
- active = true;
- while (active) {
+ while (Active()) {
int r;
uchar *b = ringBuffer->Get(r);
if (b) {
===================================================================
RCS file: ./RCS/recorder.h
retrieving revision 1.3
@@ -23,7 +23,6 @@
cRingBufferLinear *ringBuffer;
cRemux *remux;
cFileWriter *writer;
- bool active;
protected:
virtual void Activate(bool On);
virtual void Receive(uchar *Data, int Length);
===================================================================
RCS file: ./RCS/remote.c
retrieving revision 1.42
@@ -213,7 +213,6 @@
:cRemote("KBD")
,cThread("KBD remote control")
{
- active = false;
tcgetattr(STDIN_FILENO, &savedTm);
struct termios tm;
if (tcgetattr(STDIN_FILENO, &tm) == 0) {
@@ -230,7 +229,6 @@
cKbdRemote::~cKbdRemote()
{
kbdAvailable = false;
- active = false;
Cancel(3);
tcsetattr(STDIN_FILENO, TCSANOW, &savedTm);
}
@@ -261,12 +259,11 @@
void cKbdRemote::Action(void)
{
cPoller Poller(STDIN_FILENO);
- active = true;
- while (active) {
+ while (Active()) {
if (Poller.Poll(100)) {
uint64 Command = 0;
uint i = 0;
- while (active && i < sizeof(Command)) {
+ while (Active() && i < sizeof(Command)) {
uchar ch;
int r = read(STDIN_FILENO, &ch, 1);
if (r == 1) {
===================================================================
RCS file: ./RCS/remote.h
retrieving revision 1.29
@@ -81,7 +81,6 @@
class cKbdRemote : public cRemote, private cThread {
private:
- bool active;
static bool kbdAvailable;
static bool rawMode;
struct termios savedTm;
===================================================================
RCS file: ./RCS/sections.c
retrieving revision 1.11
@@ -44,7 +44,6 @@
{
shp = new cSectionHandlerPrivate;
device = Device;
- active = false;
statusCount = 0;
on = false;
waitForLock = false;
@@ -54,7 +53,6 @@
cSectionHandler::~cSectionHandler()
{
- active = false;
Cancel(3);
cFilter *fi;
while ((fi = filters.First()) != NULL)
@@ -166,9 +164,8 @@
void cSectionHandler::Action(void)
{
- active = true;
SetPriority(19);
- while (active) {
+ while (Active()) {
Lock();
if (waitForLock)
===================================================================
RCS file: ./RCS/sections.h
retrieving revision 1.4
@@ -25,7 +25,6 @@
private:
cSectionHandlerPrivate *shp;
cDevice *device;
- bool active;
int statusCount;
bool on, waitForLock;
time_t lastIncompleteSection;
===================================================================
RCS file: ./RCS/thread.c
retrieving revision 1.43
@@ -197,7 +197,7 @@
cThread::cThread(const char *Description)
{
- running = false;
+ running = active = false;
childTid = 0;
description = NULL;
SetDescription(Description);
@@ -205,6 +205,7 @@
cThread::~cThread()
{
+ Cancel(); // just in case the derived class didn't call it
free(description);
}
@@ -233,6 +234,7 @@
Thread->Action();
if (Thread->description)
dsyslog("%s thread ended (pid=%d, tid=%ld)", Thread->description, getpid(), pthread_self());
+ Thread->active = false;
Thread->running = false;
return NULL;
}
@@ -240,21 +242,21 @@
bool cThread::Start(void)
{
if (!running) {
- running = true;
+ running = active = true;
if (pthread_create(&childTid, NULL, (void *(*) (void *))&StartThread, (void *)this) == 0) {
pthread_detach(childTid); // auto-reap
pthread_setschedparam(childTid, SCHED_RR, 0);
}
else {
LOG_ERROR;
- running = false;
+ running = active = false;
return false;
}
}
return true;
}
-bool cThread::Active(void)
+bool cThread::Running(void)
{
if (running) {
//
@@ -271,7 +273,7 @@
if (err != ESRCH)
LOG_ERROR;
childTid = 0;
- running = false;
+ running = active = false;
}
else
return true;
@@ -281,10 +283,11 @@
void cThread::Cancel(int WaitSeconds)
{
+ active = false;
if (running) {
if (WaitSeconds > 0) {
for (time_t t0 = time(NULL) + WaitSeconds; time(NULL) < t0; ) {
- if (!Active())
+ if (!Running())
return;
cCondWait::SleepMs(10);
}
===================================================================
RCS file: ./RCS/thread.h
retrieving revision 1.28
@@ -76,6 +76,7 @@
friend class cThreadLock;
private:
bool running;
+ bool active;
pthread_t childTid;
cMutex mutex;
char *description;
@@ -86,13 +87,30 @@
void Lock(void) { mutex.Lock(); }
void Unlock(void) { mutex.Unlock(); }
virtual void Action(void) = 0;
+ ///< A derived cThread class must implement the code it wants to
+ ///< execute as a separate thread in this function. If this is
+ ///< a loop, it must check Active() repeatedly to see whether
+ ///< it's time to stop.
+ bool Active(void) { return active; }
+ ///< Returns false if a derived cThread object shall leave its Action()
+ ///< function.
void Cancel(int WaitSeconds = 0);
+ ///< Cancels the thread by first setting 'active' to false, so that
+ ///< the Action() loop can finish in an orderly fashion and then waiting
+ ///< up to WaitSeconds seconds for the thread to actually end. If the
+ ///< thread doesn't end by itself, it is killed.
public:
cThread(const char *Description = NULL);
+ ///< Creates a new thread.
+ ///< If Description is present, a log file entry will be made when
+ ///< the thread starts and stops. The Start() function must be called
+ ///< to actually start the thread.
virtual ~cThread();
void SetDescription(const char *Description, ...);
bool Start(void);
- bool Active(void);
+ ///< Actually starts the thread.
+ bool Running(void);
+ ///< Checks whether the thread is actually running.
static bool EmergencyExit(bool Request = false);
};
===================================================================
RCS file: ./RCS/transfer.c
retrieving revision 1.28
@@ -21,7 +21,6 @@
ringBuffer = new cRingBufferLinear(TRANSFERBUFSIZE, TS_SIZE * 2, true, "Transfer");
remux = new cRemux(VPid, APids, Setup.UseDolbyDigital ? DPids : NULL, SPids);
needsBufferReserve = Setup.UseDolbyDigital && VPid != 0 && DPids && DPids[0] != 0;
- active = false;
}
cTransfer::~cTransfer()
@@ -34,21 +33,17 @@
void cTransfer::Activate(bool On)
{
- if (On) {
- if (!active)
- Start();
- }
- else if (active) {
- active = false;
+ if (On)
+ Start();
+ else
Cancel(3);
- }
}
void cTransfer::Receive(uchar *Data, int Length)
{
- if (IsAttached() && active) {
+ if (IsAttached() && Active()) {
int p = ringBuffer->Put(Data, Length);
- if (p != Length && active)
+ if (p != Length && Active())
ringBuffer->ReportOverflow(Length - p);
return;
}
@@ -70,8 +65,7 @@
bool GotBufferReserve = false;
int RequiredBufferReserve = KILOBYTE(DvbCardWith4MBofSDRAM ? 288 : 576);
#endif
- active = true;
- while (active) {
+ while (Active()) {
#ifdef FW_NEEDS_BUFFER_RESERVE_FOR_AC3
if (needsBufferReserve && !GotBufferReserve) {
//XXX For dolby we've to fill the buffer because the firmware does
@@ -145,7 +139,6 @@
}
}
}
- active = false;
}
// --- cTransferControl ------------------------------------------------------
===================================================================
RCS file: ./RCS/transfer.h
retrieving revision 1.9
@@ -21,7 +21,6 @@
cRingBufferLinear *ringBuffer;
cRemux *remux;
bool needsBufferReserve;
- bool active;
protected:
virtual void Activate(bool On);
virtual void Receive(uchar *Data, int Length);