spark: fix race condition in cDemux

cDemux destructor was racing with Read() which did lead to all
sorts of nasty crashes because after poll returned, the dmx object
could be gone and its memory replaced with totally different things.
This commit is contained in:
Stefan Seyfried
2015-02-20 09:30:19 +01:00
committed by max_10
parent 70d7730f82
commit b67a0a7e44
2 changed files with 35 additions and 4 deletions

View File

@@ -66,11 +66,13 @@
#include <cstring>
#include <cstdio>
#include <string>
#include <OpenThreads/Mutex>
#include <OpenThreads/ScopedLock>
#include "dmx_lib.h"
#include "lt_debug.h"
/* Ugh... see comment in destructor for details... */
#include "video_lib.h"
/* needed for getSTC... */
extern cVideo *videoDecoder;
#define lt_debug(args...) _lt_debug(TRIPLE_DEBUG_DEMUX, this, args)
@@ -119,9 +121,11 @@ static const char *devname[NUM_DEMUXDEV] = {
/* did we already DMX_SET_SOURCE on that demux device? */
static bool init[NUM_DEMUXDEV] = { false, false, false };
/* uuuugly */
static int dmx_tp_count = 0;
#define MAX_TS_COUNT 1
typedef struct dmx_pdata {
int last_source;
OpenThreads::Mutex *mutex;
} dmx_pdata;
#define P ((dmx_pdata *)pdata)
cDemux::cDemux(int n)
{
@@ -137,6 +141,11 @@ cDemux::cDemux(int n)
last_measure = 0;
last_data = 0;
last_source = -1;
pdata = (void *)calloc(1, sizeof(dmx_pdata));
P->last_source = -1;
P->mutex = new OpenThreads::Mutex;
dmx_type = DMX_INVALID;
}
cDemux::~cDemux()
@@ -155,6 +164,12 @@ cDemux::~cDemux()
*/
if (dmx_type == DMX_VIDEO_CHANNEL)
videoDecoder = NULL;
/* wait until Read() has released the mutex */
(*P->mutex).lock();
(*P->mutex).unlock();
free(P->mutex);
free(pdata);
pdata = NULL;
}
bool cDemux::Open(DMX_CHANNEL_TYPE pes_type, void * /*hVideoBuffer*/, int uBufferSize)
@@ -250,6 +265,7 @@ void cDemux::Close(void)
bool cDemux::Start(bool)
{
lt_debug("%s #%d fd: %d type: %s\n", __func__, num, fd, DMX_T[dmx_type]);
if (fd < 0)
{
lt_info("%s #%d: not open!\n", __FUNCTION__, num);
@@ -261,6 +277,7 @@ bool cDemux::Start(bool)
bool cDemux::Stop(void)
{
lt_debug("%s #%d fd: %d type: %s\n", __func__, num, fd, DMX_T[dmx_type]);
if (fd < 0)
{
lt_info("%s #%d: not open!\n", __FUNCTION__, num);
@@ -282,6 +299,8 @@ int cDemux::Read(unsigned char *buff, int len, int timeout)
lt_info("%s #%d: not open!\n", __func__, num);
return -1;
}
/* avoid race in destructor: ~cDemux needs to wait until Read() returns */
OpenThreads::ScopedLock<OpenThreads::Mutex> m_lock(*P->mutex);
int rc;
int to = timeout;
struct pollfd ufds;
@@ -299,6 +318,12 @@ int cDemux::Read(unsigned char *buff, int len, int timeout)
{
retry:
rc = ::poll(&ufds, 1, to);
if (ufds.fd != fd)
{
/* Close() will set fd to -1, this is normal. Everything else is not. */
lt_info("%s:1 ========== fd has changed, %d->%d ==========\n", __func__, ufds.fd, fd);
return -1;
}
if (!rc)
{
if (timeout == 0) /* we took the emergency exit */
@@ -336,6 +361,11 @@ int cDemux::Read(unsigned char *buff, int len, int timeout)
return 0;
}
}
if (ufds.fd != fd) /* does this ever happen? and if, is it harmful? */
{ /* read(-1,...) will just return EBADF anyway... */
lt_info("%s:2 ========== fd has changed, %d->%d ==========\n", __func__, ufds.fd, fd);
return -1;
}
rc = ::read(fd, buff, len);
//fprintf(stderr, "fd %d ret: %d\n", fd, rc);