July 2008 - Wine-devel - list.winehq.org

re: Visual Runtime conflicts with the debugger?
by Dan Kegel 20 Jul '08

20 Jul '08

Markus Hitter wrote: >My pet app (Catia) works partially now. That's great! I have a copy, too, but the only thing I did with it was file a bug about the installer... >The thing most missing is, it >can't open a file. As soon as I attempt to do so, the app freezes for >a minute, then a "Visual C++ Runtime Exception" is shown. >[I tried using winedbg to track this down but it doesn't work.] Most of us use log files rather than debuggers to track this kind of thing down. You might want to try WINEDEBUG=+seh,+relay wine catia.exe > log 2>&1 and then get the shortest possible session that reproduces the bug (ideally, noninteractive; does catia take a commandline parameter to open a file?) The trick is to wade through the log file without despairing at how big it is. Tools like grep, head, tail, and in bad cases, perl or sed, are handy for this... - Dan

1 0

Re: [1/2] server: fix async read on mailslot
by Andrey Turkin 20 Jul '08

20 Jul '08

Hi Ambroz, > - > FILE_SYNCHRONOUS_IO_NONALERT ); > + read_timeout ? 0 : > FILE_SYNCHRONOUS_IO_NONALERT ); This looks incorrect. You probably want to compare to -1, not zero. Zero means reads must not block at all, and -1 means read must wait forever (not sure what zero timeout would work as expected with your patch, but it didn't anyway and I think it maybe is broken in another place) Also, please attach patches with disposition inline, not attachment (git can do that) - this make review somehow easier. Regards, Andrey Ambroz Bizjak wrote: > Hi, > > Changelog: > server: fix async read on mailslot > ------------------------------------------------------------------------ > > >

2 2

Visual Runtime conflicts with the debugger?
by Markus Hitter 20 Jul '08

20 Jul '08

Hello all, My pet app (Catia) works partially now. The thing most missing is, it can't open a file. As soon as I attempt to do so, the app freezes for a minute, then a "Visual C++ Runtime Exception" is shown. The shown dialog doesn't feature any meaningful information. So I moved from "wine" at the command line to "winedbg" to get a grip. Unfortunately, this runtime exception doesn't cause the debugger to jump in. All I get is one line: > wine: Unhandled exception 0x80000003 at address 0x7b844351 (thread > 0027), starting debugger... If I force the debugger to pay attention by typing Ctrl-C, I can even get a tiny backtrace: > Backtrace: > =>1 0xffffe40e (0x0033f260) > 2 0x00000800 (0x0033f3a0) > 3 0x7b84ab42 ReadFile+0x264(hFile=0x50, buffer=0x33f448, > bytesToRead=0x800, bytesRead=0x33fcc0, overlapped=0x0) [/home/mah/ > wine/dlls/kernel32/file.c:451] in kernel32 (0x0033f420) > 4 0x0040367b in catstart (+0x367b) (0x0033fc58) Nevertheless, the app continues to run in the GUI as before, making it impossible to check print variables or do stepping. As a third chance, I tried "winedbg --gdb". However, this stops very early: > 00000038:00000013: exception code=0x80000003 > warning: Target reported unsupported offsets: > Text=00110e20;Data=000000be;Bss=00000008 > [New Thread 19] > 0x7b8829fd in DbgBreakPoint () at ../../include/winternl.h:1828 > 1828 static inline void WINAPI DbgBreakPoint(void) { __asm__ > __volatile__("int3"); } > trace: 98 => 80 After showing this, no debugger prompt follows and a Ctrl-C is answered with a quit of both, the debugger and the app. As a last resort I tried to insert printf() statements into the source, finding me unable to repeat the numbers seen in the tiny backtrace above. bytesToRead obviously never has the value 0x800 (= 2048). So my question is: How would I go and debug such an issue? Is it even possible to set breakpoints, to get meaningful backtraces and print() statements and/or to catch Visual Whatever Runtime Exceptions? Thanks, Markus P.S.: This is a current Wine (git) on Ubuntu 64-bit. - - - - - - - - - - - - - - - - - - - Dipl. Ing. Markus Hitter http://www.jump-ing.de/

1 0

Re: Japanese conversion tests
by Dmitry Timoshkov 20 Jul '08

20 Jul '08

"Aric Stewart" <aric(a)codeweavers.com> wrote: > --- > dlls/mlang/tests/mlang.c | 173 > ++++++++++++++++++++++++++++++++++++++++++++++ > 1 files changed, 173 insertions(+), 0 deletions(-) This behaviour is not mlang specific, and should be added to dlls/kernel32/tests/codepage.c instead. -- Dmitry.

1 0

Interface fonts and locale woes
by Rafał Mużyło 20 Jul '08

20 Jul '08

I wasn't sure if I should post this to the devel or user list. A coin toss decided on this one. First of all, I'd like to praise the author of the patch at http://www.winehq.org/pipermail/wine-patches/2008-July/057033.html for "brilliance of simplicity". As I have all the fonts I needed, it annoyed me a lot that wine was displaying boxes in dialogs, instead of correct glyphs. With that patch, fonts are correctly displayed without any further tweaking. I still probably need to construct a fontconfig rule to tweak it a little bit (mainly antialiasing), but those are just details. Now, for the questions: 1. Do I understand correctly, that this rule should be for Tahoma or should it be another font ? 2. Is there any mechanism to emulate what AppLocale does in a sane way (no, LANG override does not count, it puts whole wine into a different locale, instead of only the app; while that works for most apps, it seems that some make silly assumptions based on locale, like 'what "My Documents" folder is called', also I don't want wine debug messages in a language I can't read fluently) ? Maybe it would be a good idea to expose for the user some of the changes, locale switch makes, so the user could decide which of them are needed, so it could be possible to i.e. run a program with only default ansi charset changed.

1 0

Re: [PATCH] gdi32: Glyph advances should be measured for unrotated characters.
by Dmitry Timoshkov 20 Jul '08

20 Jul '08

"Lei Zhang" <thestig(a)google.com> wrote: > Committish 85c16fc45 broke some text rendering for me. It looks like > when the font has orientation set, adv got the width of the rotated > chars, whereas the comment in struct GM says it's the width of the > unrotated chars. > > Dmitry, can you comment on the patch? It should be possible to add a test case for this. -- Dmitry.

1 0

re: [2/6] winhttp: Allow winhttp to use wininet.h with winhttp.h
by Dan Kegel 20 Jul '08

20 Jul '08

James wrote: > I thought the idea was to implement winhttp and then > implement wininet on top of winhttp. Why are you importing wininet? As of January, Hans was still talking about implementing winhttp on top of wininet ( http://www.winehq.org/pipermail/wine-devel/2008-January/062172.html http://www.winehq.org/pipermail/wine-devel/2007-August/058420.html ) It's not clear one can fully implement either wininet on top of winhttp or vice versa. We will probably need some code duplication or private interfaces eventually. Building winhttp on top of wininet lets us develop on trunk and is probably the fastest path to making a bunch of applications happy, isn't it? That's the way I'm leaning, anyway. - Dan

5 4

Re: [8/11] kernel32: Implement the timer queue thread.
by Rob Shearman 19 Jul '08

19 Jul '08

2008/7/19 Dan Hipschman <dsh(a)linux.ucla.edu>: > > This patch implements the timer thread, running callbacks, cleaning up > timers, and all that good stuff. It should be very efficient, using mostly > atomic operations and one critical section for thread safety. It would be more efficient for queues with large numbers of timers if you used the approach used by server/fd.c of keeping a list of timers sorted by when they next expire to avoid having to search through the entire list every time a timer expires. > I designed > it with thread-safety in mind, and once I got it working the way I wanted, > ran the tests in a loop a few thousand times while I ate lunch. I also did > this with the following patches and never once got a single error, hang or > crash. > > It punts on some integer-wrapping issues. If someone runs their wineserver > for 49 days they may run into that issue. I just thought it would distract > from the main content and wanted to finish everything else before I thought > about it. > > Note that the one todo_wine that got added in the tests wasn't because > something broke, it's just that the function never ran before. > > --- > dlls/kernel32/sync.c | 214 ++++++++++++++++++++++++++++++++++++++++---- > dlls/kernel32/tests/sync.c | 42 +++++++-- > 2 files changed, 231 insertions(+), 25 deletions(-) As mentioned previously, this should be implemented in ntdll. > > diff --git a/dlls/kernel32/sync.c b/dlls/kernel32/sync.c > index 8085077..4df5b44 100644 > --- a/dlls/kernel32/sync.c > +++ b/dlls/kernel32/sync.c > @@ -21,6 +21,7 @@ > #include "config.h" > #include "wine/port.h" > > +#include <assert.h> > #include <string.h> > #ifdef HAVE_UNISTD_H > # include <unistd.h> > @@ -1043,17 +1044,145 @@ BOOL WINAPI CancelWaitableTimer( HANDLE handle ) > } > > > +struct timer_queue; > struct queue_timer > { > + struct timer_queue *q; > struct list entry; > + LONG runcount; /* number of callbacks pending execution */ > + WAITORTIMERCALLBACK callback; > + PVOID param; > + DWORD period; > + ULONG flags; > + DWORD expire; > + BOOL die; /* timer should be deleted; once set, never unset */ "destroy" or "delete" might be a better name for this field. > }; > > struct timer_queue > { > RTL_CRITICAL_SECTION cs; > struct list timers; > + struct queue_timer *next; /* the next timer to expire */ > + BOOL quit; /* queue should be deleted; once set, never unset */ > + DWORD timeout; /* when the next timer expires */ > + HANDLE event; > + HANDLE thread; > }; > > +static void queue_remove_timer(struct timer_queue *q, struct queue_timer *t) > +{ > + /* We MUST hold the queue cs while calling this function. This ensures > + that we cannot queue another callback for this timer. The runcount > + being zero makes sure we don't have any already queued. */ > + assert(t->runcount == 0); > + assert(t->die); > + list_remove(&t->entry); > + if (q->next == t) > + q->next = NULL; > + HeapFree(GetProcessHeap(), 0, t); > +} > + > +static DWORD WINAPI timer_callback_wrapper(LPVOID p) > +{ > + struct queue_timer *t = (struct queue_timer *) p; This cast is unnecessary. > + struct timer_queue *q = t->q; > + > + t->callback(t->param, TRUE); > + > + RtlEnterCriticalSection(&q->cs); > + InterlockedDecrement(&t->runcount); Since you only ever access runcount while inside a critical section then you don't need to use interlocked functions to modify it. > + if (t->die && t->runcount == 0) > + { > + queue_remove_timer(q, t); > + /* Wake up the cleanup loop so it can see if all timers are gone. */ > + if (q->quit) > + SetEvent(q->event); > + } > + RtlLeaveCriticalSection(&q->cs); > + > + return 0; > +} > + > +static void queue_timer_expire(struct queue_timer *t) > +{ > + /* We MUST hold the queue cs while calling this function. */ > + if (!t->die) > + { > + ULONG flags = > + t->flags & (WT_EXECUTEINIOTHREAD | WT_EXECUTEINPERSISTENTTHREAD > + | WT_EXECUTELONGFUNCTION | WT_TRANSFER_IMPERSONATION); > + InterlockedIncrement(&t->runcount); > + QueueUserWorkItem(timer_callback_wrapper, t, flags); You should check the return value and undo the incrementing of t->runcount if it failed. However, you're also calling QueueUserWorkItem inside a critical section so you're imposing a lock ordering on the loader lock. This may cause a deadlock that may or may not exist on Windows if a timer queue function is called from an application's DllMain. > + } > + > + if (t->period) > + { > + /* FIXME: Solve this, here or in timer_queue_update. */ > + DWORD tick = GetTickCount(); > + t->expire = tick + t->period; > + if (t->expire < tick) > + ERR("next expiration wrapped\n"); > + } > + else > + t->die = TRUE; > +} > + > +static void timer_queue_update(struct timer_queue *q) > +{ > + /* We MUST hold the queue cs while calling this function. */ > + struct queue_timer *t, *temp, *next = NULL; > + DWORD tick, time = ~(DWORD) 0; This cast is unnecessary is you use "~0U" instead. > + > + LIST_FOR_EACH_ENTRY_SAFE(t, temp, &q->timers, struct queue_timer, entry) > + if (t->die) > + { > + if (t->runcount == 0) > + queue_remove_timer(q, t); > + } > + else if (t->expire < time) > + { > + time = t->expire; > + next = t; > + } > + > + tick = GetTickCount(); > + q->timeout = next ? (time < tick ? 0 : time - tick) : INFINITE; > + q->next = next; > +} > + > +static DWORD WINAPI timer_queue_thread_proc(LPVOID p) > +{ > + struct timer_queue *q = p; > + BOOL done; > + > + while (!q->quit) > + { > + DWORD ret = WaitForSingleObject(q->event, q->timeout); > + RtlEnterCriticalSection(&q->cs); > + if (ret == WAIT_TIMEOUT && q->next) > + queue_timer_expire(q->next); > + timer_queue_update(q); > + RtlLeaveCriticalSection(&q->cs); > + } > + > + done = FALSE; > + while (!done) > + { > + RtlEnterCriticalSection(&q->cs); > + timer_queue_update(q); > + if (list_count(&q->timers) == 0) > + done = TRUE; > + RtlLeaveCriticalSection(&q->cs); > + if (!done) > + WaitForSingleObject(q->event, INFINITE); > + } > + > + CloseHandle(q->event); > + CloseHandle(q->thread); > + HeapFree(GetProcessHeap(), 0, q); > + return 0; > +} > + > /*********************************************************************** > * CreateTimerQueue (KERNEL32.@) > */ > @@ -1065,15 +1194,27 @@ HANDLE WINAPI CreateTimerQueue(void) > SetLastError(ERROR_NOT_ENOUGH_MEMORY); > return NULL; > } > + > RtlInitializeCriticalSection(&q->cs); > list_init(&q->timers); > - return q; > -} > + q->next = NULL; > + q->quit = FALSE; > + q->timeout = INFINITE; > + q->event = CreateEventW(NULL, FALSE, FALSE, NULL); > + if (!q->event) > + { > + HeapFree(GetProcessHeap(), 0, q); > + return NULL; > + } > + q->thread = CreateThread(NULL, 0, timer_queue_thread_proc, q, 0, NULL); > + if (!q->thread) > + { > + CloseHandle(q->event); > + HeapFree(GetProcessHeap(), 0, q); > + return NULL; > + } It would be better to use a thread pool thread for processing timer queue since it can be reused when the timer queue is destroyed. It might then also be better to only start the timer queue thread when there are timers to process and also release the timer queue thread back to the thread pool when the last timer is removed from the queue. Also, have you thought about using one thread for all timer queues? This might not be appropriate for WT_EXECUTEINTIMERTHREAD timers, but should improve the performance of the process as a whole without affecting other timers. > > -static void queue_remove_timer(struct timer_queue *q, struct queue_timer *t) > -{ > - list_remove(&t->entry); > - HeapFree(GetProcessHeap(), 0, t); > + return q; > } > > /*********************************************************************** > @@ -1085,22 +1226,32 @@ BOOL WINAPI DeleteTimerQueueEx(HANDLE TimerQueue, HANDLE CompletionEvent) > struct queue_timer *t, *temp; > BOOL ret; > > + RtlEnterCriticalSection(&q->cs); > + LIST_FOR_EACH_ENTRY_SAFE(t, temp, &q->timers, struct queue_timer, entry) > + t->die = TRUE; > + q->quit = TRUE; > + RtlLeaveCriticalSection(&q->cs); > + SetEvent(q->event); > + > if (CompletionEvent == INVALID_HANDLE_VALUE) > { > ret = TRUE; > + WaitForSingleObject(q->thread, INFINITE); What if timer_queue_thread_proc exits before you reach here? q->thread will now be invalid (and could be re-used by another thread in the mean time). Another way of looking at it is that you don't own q after the critical section where q->quit is set. > } > else > { > + if (CompletionEvent) > + { > + /* FIXME: This should add the event to a list of events in the > + queue that it will signal when done. */ > + FIXME("returning and signaling events unimplemented; waiting instead\n"); > + WaitForSingleObject(q->thread, INFINITE); > + SetEvent(CompletionEvent); > + } > ret = FALSE; > SetLastError(ERROR_IO_PENDING); > } > > - RtlEnterCriticalSection(&q->cs); > - LIST_FOR_EACH_ENTRY_SAFE(t, temp, &q->timers, struct queue_timer, entry) > - queue_remove_timer(q, t); > - RtlLeaveCriticalSection(&q->cs); > - > - HeapFree(GetProcessHeap(), 0, q); > return ret; > } > > @@ -1123,19 +1274,50 @@ BOOL WINAPI CreateTimerQueueTimer( PHANDLE phNewTimer, HANDLE TimerQueue, > { > struct timer_queue *q = TimerQueue; > struct queue_timer *t = HeapAlloc(GetProcessHeap(), 0, sizeof *t); > + BOOL ret; > + > if (!t) > { > SetLastError(ERROR_NOT_ENOUGH_MEMORY); > return FALSE; > } > - FIXME(": Timer expiration unimplemented\n"); > + t->q = q; > + t->runcount = 0; > + t->callback = Callback; > + t->param = Parameter; > + t->period = Period; > + t->flags = Flags; > + if (Flags & WT_EXECUTEINTIMERTHREAD) > + FIXME("WT_EXECUTEINTIMERTHREAD unimplemented\n"); If you use an expire list in a local variable then you won't need locking when processing the expired timers and so this will be trivial to implement. > + { > + /* FIXME: Solve this, here or in timer_queue_update. */ > + DWORD tick = GetTickCount(); > + t->expire = tick + DueTime; > + if (t->expire < tick) > + ERR("due time wrapped\n"); > + } > + t->die = FALSE; > > + ret = TRUE; > RtlEnterCriticalSection(&q->cs); > - list_add_tail(&q->timers, &t->entry); > + if (q->quit) > + ret = FALSE; > + else > + list_add_tail(&q->timers, &t->entry); > RtlLeaveCriticalSection(&q->cs); > > - *phNewTimer = t; > - return TRUE; > + if (ret) > + { > + SetEvent(q->event); > + *phNewTimer = t; > + } > + else > + { > + SetLastError(ERROR_INVALID_HANDLE); > + HeapFree(GetProcessHeap(), 0, t); > + } > + > + return ret; > } > > /*********************************************************************** Quite a few comments, but the patches are of a good general quality, so well done. -- Rob Shearman

2 1

Re: [Gdiplus 02/15] Implement GdipCreateRegion
by Huw Davies 19 Jul '08

19 Jul '08

Adam Petaccia wrote: > @@ -226,6 +277,11 @@ GpStatus WINGDIPAPI GdipSetEmpty(GpRegion*region) > > if(!(calls++)) > FIXME("not implemented\n"); > + TRACE("%p\n", region); > + > + GdipDeleteRegion(region); > + GdipCreateRegion(&region); > + region->node->type = RegionDataEmptyRect; > > return NotImplemented; > } This can't work. You don't want to free the memory pointed to by region, simply free the nodes underneath it. Huw.

2 1

Re: [5/11] kernel32: Implement Create/DeleteTimerQueue[Ex].
by Rob Shearman 19 Jul '08

19 Jul '08

2008/7/19 Dan Hipschman <dsh(a)linux.ucla.edu>: > --- > dlls/kernel32/sync.c | 40 ++++++++++++++++++++++++++++++++++------ > dlls/kernel32/tests/sync.c | 11 +++-------- > 2 files changed, 37 insertions(+), 14 deletions(-) These functions should be implemented in ntdll instead of kernel32. I would suggest dlls/ntdll/threadpool.c. -- Rob Shearman

1 0