Opened 16 years ago

Closed 16 years ago

Last modified 2 years ago

#1684 closed defect (invalid)

SmushPlayer can crash while using threaded timer

Reported by: SF/uweryssel Owned by: aquadran
Priority: normal Component: Engine: SCUMM
Keywords: Cc:
Game:

Description

The SmushPlayer class isn't secure, while using a threaded timer.

When the SmushPlayer object is destructed it can be happen that the timer callback is still running, because the callback is launched by another thread.

Removing the callback in the destructor can't remove the problem, because it only prevents new callback runs and not stop the current one.

My solution is to wait 500ms after the removeTimerProc, so all running callbacks can be processed. But this is not the best solution.

I've seen the same bug in IMuseDigital class too.

Ticket imported from: #996674. Ticket imported from: bugs/1684.

Change History (6)

comment:1 by sev-, 16 years ago

Timers were in discussion between Fingolfin and aquadran couple days ago with proposed new approach by aquadran. I think he'd better handle this bug. Reassigned.

comment:2 by sev-, 16 years ago

Owner: changed from sev- to aquadran

comment:3 by fingolfin, 16 years ago

I can't quite follow the bug report.

First off, what do you mean with "threaded timer" ? If you are talking about aquadrans experimental patch, reporting a bug on it here is bogus. If you are talking about some custom hack of your own or the morphos timer code: please state that clearly.

If you are talking about the current default implementation of the Timer class: I don't quite understand how the problem you describe could arise there. It uses a mutex in both installTimerProc and removeTimerProc.

Hence removeTimerProc will block if the callback is currently running (even if it is running in a separate thread). So it doesn't "stop" the current callback run, but it does wait for it to exit (which could of course lead to a deadlock should that timer proc "hang", but that would be a bad bad thing anyway).

Could you please explain a bit more what exactly you believe is going wrong there?

comment:4 by fingolfin, 16 years ago

Resolution: invalid
Status: newclosed

comment:5 by fingolfin, 16 years ago

Bug report looks bogus, and submitter never explained it properly, so closing this.

comment:6 by digitall, 2 years ago

Component: --Unset--Engine: SCUMM
Note: See TracTickets for help on using tickets.