Re: [PATCH 1/2] winmm: Avoid using SuspendThread, it can hang Wine.
On 17 February 2012 16:14, wrote: >>I don't see any apps in that bug report, it's only about a test failure. >>Do you have real apps that trigger the problem in normal usage? > > The two apps from #26997 #23756 use midiStream with DCB_CALLBACK_FUNCTION. > They need patch 1+2. You mean #26697 Alex
Re: [PATCH 1/2] winmm: Avoid using SuspendThread, it can hang Wine.
>I don't see any apps in that bug report, it's only about a test failure. >Do you have real apps that trigger the problem in normal usage? The two apps from #26997 #23756 use midiStream with DCB_CALLBACK_FUNCTION. They need patch 1+2. I've not retested ff7 from bug #9220 which uses midiStream too. The apps that would benefit from patch 1/2 alone are those that use midiStream with one of DCB_NULL, WINDOW or EVENT. I've not investigated which MIDI apps are of the required kind or if they use mcimidi, midiOut directly or midiStream with DCB_FUNCTION. Regards, Jörg Höhle
Re: [PATCH 1/2] winmm: Avoid using SuspendThread, it can hang Wine.
joerg-cyril.hoe...@t-systems.com writes: > Hi, > >>this Fixes bug #28388 > > I think it is not a good idea to defer this patch until after 1.4.0. > > Every app using midiStreamOpen eventually calls SuspendThread > which is simply a time bomb on a dual core machine. There are many > more multi-core machines now, so the situation degrades. I don't see any apps in that bug report, it's only about a test failure. Do you have real apps that trigger the problem in normal usage? -- Alexandre Julliard julli...@winehq.org
[PATCH 1/2] winmm: Avoid using SuspendThread, it can hang Wine.
Hi, >this Fixes bug #28388 I think it is not a good idea to defer this patch until after 1.4.0. Every app using midiStreamOpen eventually calls SuspendThread which is simply a time bomb on a dual core machine. There are many more multi-core machines now, so the situation degrades. Dan Kegel wrote: >With WINEDEBUG=warn+heap, it happens a lot sooner (first run: first try; >second run: sixth try). As noted in the bug report, SuspendThread can hit Wine amid memory management, breaking havoc. Users won't know who to blame for random lock-ups and complain like I did that Wine isn't able to generate a backtrace. Patch 1/2 can be applied stand alone. It gets rid of SuspendThread. It's been a pleasure to use patch 2/2 and hear apps play midiStream music without trouble. I wouldn't mind if you defer that for 1.4.1. After all, this is a feature that has been lacking in Wine for the last 10 years, so people can wait a little more... Regards, Jörg Höhle
Re: [PATCH 1/2] winmm: Avoid using SuspendThread, it can hang Wine.
joerg-cyril.hoe...@t-systems.com wrote: > Michael Stefaniuc wrote: >> See that as an opportunity to clean up the whitespace issues in those 120 >> lines ;) > Hmm, what was the latest word on reformatting, if any? > Until now I've been careful to preserve the surrounding formatting, even > Though winmm&mci look very different from mmdevapi. > > Do you suggest to remove all TAB from lines that I touch, even if only > re-indenting? That. And getting rid of trailing/dangling whitespace, placing the whitespace consistently around operators, etc. bye michael
Re: [PATCH 1/2] winmm: Avoid using SuspendThread, it can hang Wine.
Michael Stefaniuc wrote: >See that as an opportunity to clean up the whitespace issues in those 120 >lines ;) Hmm, what was the latest word on reformatting, if any? Until now I've been careful to preserve the surrounding formatting, even Though winmm&mci look very different from mmdevapi. Do you suggest to remove all TAB from lines that I touch, even if only re-indenting? Regards, Jörg Höhle
Re: [PATCH 1/2] winmm: Avoid using SuspendThread, it can hang Wine.
joerg-cyril.hoe...@t-systems.com wrote:
> Alexandre Julliard wrote:
>> Please stop abusing goto, write a proper loop instead.
> No problem. The idea was to minimize the patch and show what really changed.
> Adding a while loop will indent all 120 lines of code in between { }.
See that as an opportunity to clean up the whitespace issues in those
120 lines ;). Or of course to split the code out into helper functions.
bye
michael
Re: [PATCH 1/2] winmm: Avoid using SuspendThread, it can hang Wine.
Alexandre Julliard wrote:
>Please stop abusing goto, write a proper loop instead.
No problem. The idea was to minimize the patch and show what really changed.
Adding a while loop will indent all 120 lines of code in between { }.
Regards,
Jörg Höhle
Re: [PATCH 1/2] winmm: Avoid using SuspendThread, it can hang Wine.
joerg-cyril.hoe...@t-systems.com writes:
> @@ -1006,7 +1008,9 @@ static BOOL
> MMSYSTEM_MidiStream_MessageHandler(WINE_MIDIStream* lpMidiStrm, LPWI
> LPMIDIHDRlpMidiHdr;
> LPMIDIHDR* lpmh;
> LPBYTE lpData;
> +BOOL paused = FALSE;
>
> +still_paused:
> switch (msg->message) {
> case WM_QUIT:
> SetEvent(lpMidiStrm->hEvent);
> @@ -1028,6 +1032,16 @@ static BOOL
> MMSYSTEM_MidiStream_MessageHandler(WINE_MIDIStream* lpMidiStrm, LPWI
> }
> lpMidiStrm->lpMidiHdr = 0;
> SetEvent(lpMidiStrm->hEvent);
> + return TRUE;
> +case WINE_MSM_RESUME:
> + /* FIXME: send out cc64 0 (turn off sustain pedal) on every channel */
> + lpMidiStrm->dwStartTicks = GetTickCount() - lpMidiStrm->dwPositionMS;
> + SetEvent(lpMidiStrm->hEvent);
> + return TRUE;
> +case WINE_MSM_PAUSE:
> + /* FIXME: send out cc64 0 (turn off sustain pedal) on every channel */
> + paused = TRUE;
> + SetEvent(lpMidiStrm->hEvent);
> break;
> case WINE_MSM_HEADER:
> /* sets initial tick count for first MIDIHDR */
> @@ -1114,6 +1128,10 @@ static BOOL
> MMSYSTEM_MidiStream_MessageHandler(WINE_MIDIStream* lpMidiStrm, LPWI
> FIXME("Unknown message %d\n", msg->message);
> break;
> }
> +if (paused) {
> + GetMessageA(msg, 0, 0, 0);
> + goto still_paused;
> +}
Please stop abusing goto, write a proper loop instead.
--
Alexandre Julliard
julli...@winehq.org
