Graphlcd crashes with segfault when stopping an MP3 replay because it calls cMP3Control::GetIndex after the MP3 player object is already destroyed.
Reason: MP3 plugin signals end of replay too late. The attaches patch fixes it.
=================================================================== --- mp3.c (Revision 228) +++ mp3.c (Arbeitskopie) @@ -261,10 +261,10 @@
cMP3Control::~cMP3Control() { + cStatus::MsgReplaying(this, NULL); delete lastMode; Hide(); Stop(); - cStatus::MsgReplaying(this, NULL); }
void cMP3Control::Stop(void)
On 12 Mar 2005 Wolfgang Fritz wolfgang.fritz@gmx.net wrote:
Graphlcd crashes with segfault when stopping an MP3 replay because it calls cMP3Control::GetIndex after the MP3 player object is already destroyed.
Reason: MP3 plugin signals end of replay too late. The attaches patch fixes it.
Ok, I changed the location, but I think that the real flaw is in VDRs player.h where the "player" var isn't checked before access.
Regards.
Stefan Huelswitt wrote:
On 12 Mar 2005 Wolfgang Fritz wolfgang.fritz@gmx.net wrote:
Graphlcd crashes with segfault when stopping an MP3 replay because it calls cMP3Control::GetIndex after the MP3 player object is already destroyed.
Reason: MP3 plugin signals end of replay too late. The attaches patch fixes it.
Ok, I changed the location, but I think that the real flaw is in VDRs player.h where the "player" var isn't checked before access.
If tested this (temporarily inserted a test for player == 0 in player.h), but it didn't help.
maybe
void cMP3Control::Stop(void) { delete player; player=0; mgr->Halt(); mgr->Flush(); //XXX remove later }
has a concurrency problem? Who guarantees that no other thread interrupts the mp3 thread while is in the destructor?
Wouldn't be something like this a little bit safer:
void cMP3Control::Stop(void) { cMP3player *p = player; player = 0; delete p; mgr->Halt(); mgr->Flush(); //XXX remove later }
There seem to be more concurrency problems which crop up with the graphlcd plugin. In rare cases I get a segfault when exiting a normal vdr replay.
Wolfgang
Regards.
On 15 Mar 2005 Wolfgang Fritz wolfgang.fritz@gmx.net wrote:
Stefan Huelswitt wrote:
Ok, I changed the location, but I think that the real flaw is in VDRs player.h where the "player" var isn't checked before access.
If tested this (temporarily inserted a test for player == 0 in player.h), but it didn't help.
maybe
void cMP3Control::Stop(void) { delete player; player=0; mgr->Halt(); mgr->Flush(); //XXX remove later }
has a concurrency problem? Who guarantees that no other thread interrupts the mp3 thread while is in the destructor?
Wouldn't be something like this a little bit safer:
void cMP3Control::Stop(void) { cMP3player *p = player; player = 0; delete p; mgr->Halt(); mgr->Flush(); //XXX remove later }
There seem to be more concurrency problems which crop up with the graphlcd plugin. In rare cases I get a segfault when exiting a normal vdr replay.
Of course it's a concurrency problem.
From the fact that there is no mutex protection for the player
var (neither in MP3 plugin nor in any other player code), I would guess that the cControl API was designed for accesses from the VDR foreground thread only (no concurrency problem in this case). If graphlcd is running on another thread, this could be the explanation...
Regards.
Stefan Huelswitt wrote:
On 15 Mar 2005 Wolfgang Fritz wolfgang.fritz@gmx.net wrote:
Stefan Huelswitt wrote:
Ok, I changed the location, but I think that the real flaw is in VDRs player.h where the "player" var isn't checked before access.
If tested this (temporarily inserted a test for player == 0 in player.h), but it didn't help.
maybe
void cMP3Control::Stop(void) { delete player; player=0; mgr->Halt(); mgr->Flush(); //XXX remove later }
has a concurrency problem? Who guarantees that no other thread interrupts the mp3 thread while is in the destructor?
Wouldn't be something like this a little bit safer:
void cMP3Control::Stop(void) { cMP3player *p = player; player = 0; delete p; mgr->Halt(); mgr->Flush(); //XXX remove later }
There seem to be more concurrency problems which crop up with the graphlcd plugin. In rare cases I get a segfault when exiting a normal vdr replay.
Of course it's a concurrency problem.
From the fact that there is no mutex protection for the player
var (neither in MP3 plugin nor in any other player code), I would guess that the cControl API was designed for accesses from the VDR foreground thread only (no concurrency problem in this case).
That's right - a player is not supposed to be accessed from anything else than the foreground thread.
Klaus
If graphlcd is running on another thread, this could be the explanation...
Regards.
Klaus Schmidinger wrote:
Stefan Huelswitt wrote:
On 15 Mar 2005 Wolfgang Fritz wolfgang.fritz@gmx.net wrote:
Stefan Huelswitt wrote:
Ok, I changed the location, but I think that the real flaw is in VDRs player.h where the "player" var isn't checked before access.
If tested this (temporarily inserted a test for player == 0 in player.h), but it didn't help.
maybe
void cMP3Control::Stop(void) { delete player; player=0; mgr->Halt(); mgr->Flush(); //XXX remove later }
has a concurrency problem? Who guarantees that no other thread interrupts the mp3 thread while is in the destructor?
Wouldn't be something like this a little bit safer:
void cMP3Control::Stop(void) { cMP3player *p = player; player = 0; delete p; mgr->Halt(); mgr->Flush(); //XXX remove later }
There seem to be more concurrency problems which crop up with the graphlcd plugin. In rare cases I get a segfault when exiting a normal vdr replay.
Of course it's a concurrency problem.
From the fact that there is no mutex protection for the player
var (neither in MP3 plugin nor in any other player code), I would guess that the cControl API was designed for accesses from the VDR foreground thread only (no concurrency problem in this case).
That's right - a player is not supposed to be accessed from anything else than the foreground thread.
As far I can see, cControl::GetIndex() is the only method graphlcd calls in its own thread context to get periodic information about the replay progress (for updating its progress bar display). What would be the correct way to get that information?
Wolfgang
Klaus
If graphlcd is running on another thread, this could be the explanation...
Regards.
Wolfgang Fritz wrote:
Klaus Schmidinger wrote:
Stefan Huelswitt wrote:
On 15 Mar 2005 Wolfgang Fritz wolfgang.fritz@gmx.net wrote:
Stefan Huelswitt wrote:
Ok, I changed the location, but I think that the real flaw is in VDRs player.h where the "player" var isn't checked before access.
If tested this (temporarily inserted a test for player == 0 in player.h), but it didn't help.
maybe
void cMP3Control::Stop(void) { delete player; player=0; mgr->Halt(); mgr->Flush(); //XXX remove later }
has a concurrency problem? Who guarantees that no other thread interrupts the mp3 thread while is in the destructor?
Wouldn't be something like this a little bit safer:
void cMP3Control::Stop(void) { cMP3player *p = player; player = 0; delete p; mgr->Halt(); mgr->Flush(); //XXX remove later }
There seem to be more concurrency problems which crop up with the graphlcd plugin. In rare cases I get a segfault when exiting a normal vdr replay.
Of course it's a concurrency problem.
From the fact that there is no mutex protection for the player
var (neither in MP3 plugin nor in any other player code), I would guess that the cControl API was designed for accesses from the VDR foreground thread only (no concurrency problem in this case).
That's right - a player is not supposed to be accessed from anything else than the foreground thread.
As far I can see, cControl::GetIndex() is the only method graphlcd calls in its own thread context to get periodic information about the replay progress (for updating its progress bar display). What would be the correct way to get that information?
Wolfgang
I guess the correct way to do this would be to introduce a dedicated function in cStatus. Just grabbing into the cControl from another thread is not the right thing to do.
Klaus
-
Klaus Schmidinger -
s.huelswitt＠gmx.de -
Wolfgang Fritz