System calls without error checks in w32

System calls without error checks in w32

[Lennart Borgman]

There is a lot of system calls in the w32 without error checking. This
makes things harder than they should be. How about adding something
like

    DebPrint (("error w32_creatwindow.AjdustRectangle => %d\n",
GetLastError ()));

to all of them? Is there anyone who has anything against this?

Re: System calls without error checks in w32

[Eli Zaretskii]

> From: Lennart Borgman <lennart.borgman@gmail.com>
> Date: Sat, 29 May 2010 04:38:24 +0200
> 
> There is a lot of system calls in the w32 without error checking.

??? "A lot"?  Please give examples of those.

> How about adding something like
> 
>     DebPrint (("error w32_creatwindow.AjdustRectangle => %d\n",
> GetLastError ()));
> 
> to all of them? Is there anyone who has anything against this?

What for?

Re: System calls without error checks in w32

[Lennart Borgman]

On Sat, May 29, 2010 at 7:43 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Lennart Borgman <lennart.borgman@gmail.com>
>> Date: Sat, 29 May 2010 04:38:24 +0200
>>
>> There is a lot of system calls in the w32 without error checking.
>
> ??? "A lot"?  Please give examples of those.


There is a lot of them in w32fns.c. I expect the same for other files
with w32 system calls.

Here are some I found:

CreateFontIndirect
DestroyCaret
ExtTextOut
GetForeGroundWindow
GetKeyboardState
GetKeyboardState
GetModuleHandle
GetProcAddress
GetTextExtentPoint32
GetWindowPlacement
GetWindowRect
KillTimer
LocalFree
PostMessage
PostThreadMessage
RegisterHotKey
ReleaseCapture
SelectObject
SetWindowLong

A few of them does not return errors, but return value may have to be
checked anyway. I probably missed some (and did I add the
GetWindowPlacement code?).


>> How about adding something like
>>
>>     DebPrint (("error w32_creatwindow.AjdustRectangle => %d\n",
>> GetLastError ()));
>>
>> to all of them? Is there anyone who has anything against this?
>
> What for?


I have for several years complained about frequent crashes. They are
quite irritating actually.

I have to reboot quite often too, maybe because of errors in the
system calls (some resource is leaking).

Menus often stops working. Difficult to track down, and even more so
without knowing if some bad system calls where involved.

Emacs can totally hang. Or it can loop without yeilding. At least the
first one can be a bad system call.

Re: System calls without error checks in w32

[Eli Zaretskii]

> From: Lennart Borgman <lennart.borgman@gmail.com>
> Date: Sat, 29 May 2010 20:42:40 +0200
> Cc: emacs-devel@gnu.org
> 
> CreateFontIndirect
> DestroyCaret
> ExtTextOut
> GetForeGroundWindow
> GetKeyboardState
> GetKeyboardState
> GetModuleHandle
> GetProcAddress
> GetTextExtentPoint32
> GetWindowPlacement
> GetWindowRect
> KillTimer
> LocalFree
> PostMessage
> PostThreadMessage
> RegisterHotKey
> ReleaseCapture
> SelectObject
> SetWindowLong
> 
> A few of them does not return errors, but return value may have to be
> checked anyway.

If you can suggest what to do when any of these fails, please do.

> >>     DebPrint (("error w32_creatwindow.AjdustRectangle => %d\n",
> >> GetLastError ()));
> >>
> >> to all of them? Is there anyone who has anything against this?
> >
> > What for?
> 
> 
> I have for several years complained about frequent crashes. They are
> quite irritating actually.
> 
> I have to reboot quite often too, maybe because of errors in the
> system calls (some resource is leaking).
> 
> Menus often stops working. Difficult to track down, and even more so
> without knowing if some bad system calls where involved.
> 
> Emacs can totally hang. Or it can loop without yeilding. At least the
> first one can be a bad system call.

I don't see how any of these problems could be solved by using
DebPrint.

Re: System calls without error checks in w32

[Lennart Borgman]

On Sat, May 29, 2010 at 9:35 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Lennart Borgman <lennart.borgman@gmail.com>
>> Date: Sat, 29 May 2010 20:42:40 +0200
>> Cc: emacs-devel@gnu.org
>>
>> CreateFontIndirect
>> DestroyCaret
>> ExtTextOut
>> GetForeGroundWindow
>> GetKeyboardState
>> GetKeyboardState
>> GetModuleHandle
>> GetProcAddress
>> GetTextExtentPoint32
>> GetWindowPlacement
>> GetWindowRect
>> KillTimer
>> LocalFree
>> PostMessage
>> PostThreadMessage
>> RegisterHotKey
>> ReleaseCapture
>> SelectObject
>> SetWindowLong
>>
>> A few of them does not return errors, but return value may have to be
>> checked anyway.


I looked a bit more. There are many more that could be checked, but
are not checked now.


> If you can suggest what to do when any of these fails, please do.


I have already made the suggestion below. I do not mean adding it
everywhere where checks are missing, that would be a lot of job in the
current state of the code. However adding this kind of output when
hunting for a problem could be very helpful.


>> >>     DebPrint (("error w32_creatwindow.AjdustRectangle => %d\n",
>> >> GetLastError ()));
>> >>
>> >> to all of them? Is there anyone who has anything against this?
>> >
>> > What for?
>>
>>
>> I have for several years complained about frequent crashes. They are
>> quite irritating actually.
>>
>> I have to reboot quite often too, maybe because of errors in the
>> system calls (some resource is leaking).
>>
>> Menus often stops working. Difficult to track down, and even more so
>> without knowing if some bad system calls where involved.
>>
>> Emacs can totally hang. Or it can loop without yeilding. At least the
>> first one can be a bad system call.
>
> I don't see how any of these problems could be solved by using
> DebPrint.


Why not? The latest crash fixes I sent in was about missing checks.

Re: System calls without error checks in w32

[Eli Zaretskii]

> From: Lennart Borgman <lennart.borgman@gmail.com>
> Date: Sat, 29 May 2010 22:02:24 +0200
> Cc: emacs-devel@gnu.org
> 
> > If you can suggest what to do when any of these fails, please do.
> 
> 
> I have already made the suggestion below.

How in the world outputting an error message would help here?

Re: System calls without error checks in w32

[Lennart Borgman]

On Sat, May 29, 2010 at 11:25 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Lennart Borgman <lennart.borgman@gmail.com>
>> Date: Sat, 29 May 2010 22:02:24 +0200
>> Cc: emacs-devel@gnu.org
>>
>> > If you can suggest what to do when any of these fails, please do.
>>
>>
>> I have already made the suggestion below.
>
> How in the world outputting an error message would help here?

You mean instead of fixing the error directly? It is just a first step
of course.

The error message might also be interesting in itself. It can for
example tell about resources lack etc. The API descriptions does not
always tell the whole story.

Re: System calls without error checks in w32

[Eli Zaretskii]

> From: Lennart Borgman <lennart.borgman@gmail.com>
> Date: Sat, 29 May 2010 23:30:21 +0200
> Cc: emacs-devel@gnu.org
> 
> On Sat, May 29, 2010 at 11:25 PM, Eli Zaretskii <eliz@gnu.org> wrote:
> >> From: Lennart Borgman <lennart.borgman@gmail.com>
> >> Date: Sat, 29 May 2010 22:02:24 +0200
> >> Cc: emacs-devel@gnu.org
> >>
> >> > If you can suggest what to do when any of these fails, please do.
> >>
> >>
> >> I have already made the suggestion below.
> >
> > How in the world outputting an error message would help here?
> 
> You mean instead of fixing the error directly? It is just a first step
> of course.
> 
> The error message might also be interesting in itself. It can for
> example tell about resources lack etc. The API descriptions does not
> always tell the whole story.

Checking values returned by system calls is necessary to avoid working
with invalid objects returned by a failed system call.  I thought you
were talking about this.

But it turns out you are talking about something else altogether: how
to debug possible failures in some APIs.  That is a different story
and it requires different solutions.  Please take a look at xdisp.c
and friends, which have special debugging code inside
#ifdef GLYPH_DEBUG.

In any case, the solution you suggest is not a good one, because it
obfuscates the source code and makes it harder to read and understand.

Re: System calls without error checks in w32

[Lennart Borgman]

On Sun, May 30, 2010 at 5:02 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>
> In any case, the solution you suggest is not a good one, because it
> obfuscates the source code and makes it harder to read and understand.


Are you saying that checking the return values of system calls should
be avoided because it makes the code to difficult to read?

I can't imagine you mean that. What do you mean? Exactly what in my
suggestion makes the code to hard to read? How can the check be made
more readable? (I can't see how GLYPH_DEBUG style handling of it
applies here.)

Remember that what I suggest is basically

   if (bad_return_value) DebPrint (("error this_function.ApiFunction
=> %d", bad_return_value));

It can be visually more appealing with a macro, something like

   W32DEBPRINT (this_function, ApiFunction, bad_return_value, 0);

Is that what you mean?

This is of course what I meant, but my C fu is low, and so my C
language. So we might misunderstand each other.

Re: System calls without error checks in w32

[Eli Zaretskii]

> From: Lennart Borgman <lennart.borgman@gmail.com>
> Date: Sun, 30 May 2010 05:26:32 +0200
> Cc: emacs-devel@gnu.org
> 
> On Sun, May 30, 2010 at 5:02 AM, Eli Zaretskii <eliz@gnu.org> wrote:
> >
> > In any case, the solution you suggest is not a good one, because it
> > obfuscates the source code and makes it harder to read and understand.
> 
> 
> Are you saying that checking the return values of system calls should
> be avoided because it makes the code to difficult to read?

No.

> Remember that what I suggest is basically
> 
>    if (bad_return_value) DebPrint (("error this_function.ApiFunction
> => %d", bad_return_value));

What purpose does this serve?  This isn't "checking return values for
errors", because this has no effect for the majority of users who
don't run Emacs under a debugger.  Their Emacs will still crash if it
hits bad_return_value.  I thought you were arguing for making the
Emacs code more robust in the face of possible failures of the APIs it
calls.

If you want more debugging aids, I don't object, but then I have no
interest in such additions, either.  Typically, such additions are
only good as long as you debug some specific problem, so keeping them
in the code after that is waste of bytes.

>    W32DEBPRINT (this_function, ApiFunction, bad_return_value, 0);

This is even worse, because it's impossible to understand what this
does without looking up the magical W32DEBPRINT macro.

Re: System calls without error checks in w32

[Lennart Borgman]

On Sun, May 30, 2010 at 7:36 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>>
>> On Sun, May 30, 2010 at 5:02 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>> >
>> > In any case, the solution you suggest is not a good one, because it
>> > obfuscates the source code and makes it harder to read and understand.
>>
>>
>> Are you saying that checking the return values of system calls should
>> be avoided because it makes the code to difficult to read?
>
> No.

Fine.

>> Remember that what I suggest is basically
>>
>>    if (bad_return_value) DebPrint (("error this_function.ApiFunction
>> => %d", bad_return_value));
>
> What purpose does this serve?  This isn't "checking return values for
> errors", because this has no effect for the majority of users who
> don't run Emacs under a debugger.


The debugger of course does not have an effect on those users. But
what do you want to say?

My intention is to get a chance to know why a system call failed. That
is a way to find those errors that are a bit harder to find than those
you may see directly by inspecting the code.


> Their Emacs will still crash if it
> hits bad_return_value.  I thought you were arguing for making the
> Emacs code more robust in the face of possible failures of the APIs it
> calls.


Yes, of course, that should be done at the same time.


> If you want more debugging aids, I don't object, but then I have no
> interest in such additions, either.  Typically, such additions are
> only good as long as you debug some specific problem, so keeping them
> in the code after that is waste of bytes.


It seems like you think that problems with system calls are just like
any other problem in the code. For problem in Emacs code itself your
strategy here is of course good.

System calls, however is like a black box. You have to follow the
rules for calling the black box, but do not know exactly what is
inside, you can only watch the outcome to learn more.

One example of this is when I see Emacs going totally weird. That
happens quite often and I am not sure what is happening. It might in
my cases be problems with edebug or it might be problems with the
system calls that makes the depends on resource exhaustion in the
black system box. To be able to fix it I need some kind of logging of
the system calls.

So I really prefer that those debug calls for check system calls are
there permanently.


>>    W32DEBPRINT (this_function, ApiFunction, bad_return_value, 0);
>
> This is even worse, because it's impossible to understand what this
> does without looking up the magical W32DEBPRINT macro.


I think it has some advantages, actually. It is easy to see and
distinguish from the normal code. It can be a null op in non-debugging
compilations.

And it is very easy to understand immediately once you have looked up
the magical W32DEBPRINT - which is quite easy to do with Emacs tools.

But maybe you meant something else?

Re: System calls without error checks in w32

[Juanma Barranquero]

On Sun, May 30, 2010 at 20:02, Lennart Borgman
<lennart.borgman@gmail.com> wrote:

> My intention is to get a chance to know why a system call failed. That
> is a way to find those errors that are a bit harder to find than those
> you may see directly by inspecting the code.

If you have DebPrint for every system call, and some of them return
trivial errors, debugging will sometimes be harder just because of the
noise.

> System calls, however is like a black box. You have to follow the
> rules for calling the black box, but do not know exactly what is
> inside, you can only watch the outcome to learn more.

I think you'll see that many system calls won't ever return an error
(because the error conditions are pretty infrequent). Some of them
will, for example system calls related to file manipulation (because
of nonexistent or nonaccesible files, etc.); it would be interesting
to know how many of those don't have already checking in place.

> One example of this is when I see Emacs going totally weird. That
> happens quite often and I am not sure what is happening. It might in
> my cases be problems with edebug or it might be problems with the
> system calls that makes the depends on resource exhaustion in the
> black system box.

Does that happen to you with your patched Emacs, or unpatched? Because
I see occasional crashes, but not nearly as frequently as it seems to
happen to you (not by a loooong shot), and I have not seem "weird"
behavior (unresponsive Emacs, looping, etc.) perhaps for years.

> To be able to fix it I need some kind of logging of
> the system calls.

Because you somehow assume that the trouble is likely related to bad
system calls; most Emacs bugs aren't.

>> This is even worse, because it's impossible to understand what this
>> does without looking up the magical W32DEBPRINT macro.
>
>
> I think it has some advantages, actually. It is easy to see and
> distinguish from the normal code. It can be a null op in non-debugging
> compilations.

IMHO is horribly ugly. If you're going to have error checks, better to
have them on the clear.

    Juanma

Re: System calls without error checks in w32

[Lennart Borgman]

On Sun, May 30, 2010 at 9:24 PM, Juanma Barranquero <lekktu@gmail.com> wrote:
> On Sun, May 30, 2010 at 20:02, Lennart Borgman
> <lennart.borgman@gmail.com> wrote:
>
>> My intention is to get a chance to know why a system call failed. That
>> is a way to find those errors that are a bit harder to find than those
>> you may see directly by inspecting the code.
>
> If you have DebPrint for every system call, and some of them return
> trivial errors, debugging will sometimes be harder just because of the
> noise.

Yes, my intention was not to add them everywhere, only on interesting places.

> I think you'll see that many system calls won't ever return an error
> (because the error conditions are pretty infrequent). Some of them
> will, for example system calls related to file manipulation (because
> of nonexistent or nonaccesible files, etc.); it would be interesting
> to know how many of those don't have already checking in place.

On a quick look I got the impression that file calls are better
handled than GUI system calls.

>> One example of this is when I see Emacs going totally weird. That
>> happens quite often and I am not sure what is happening. It might in
>> my cases be problems with edebug or it might be problems with the
>> system calls that makes the depends on resource exhaustion in the
>> black system box.
>
> Does that happen to you with your patched Emacs, or unpatched?

It happens to me with my patched Emacs because that is the one I am
using. I only use unpatched Emacs for bug testing etc.

> Because
> I see occasional crashes, but not nearly as frequently as it seems to
> happen to you (not by a loooong shot), and I have not seem "weird"
> behavior (unresponsive Emacs, looping, etc.) perhaps for years.

All I know is that some system resources seems to be exhausted. What I
directly can see without digging is that GDI are exhausted (these are
shown in Windows Task Manager). explorer.exe are owning a lot of them,
which I believe means it is system windows like menus that are badly
handled. However if Emacs is the culprit or windows itself I do not
know. (I am using sticky keys and I think it is related to that.)

>> To be able to fix it I need some kind of logging of
>> the system calls.
>
> Because you somehow assume that the trouble is likely related to bad
> system calls; most Emacs bugs aren't.


That is surely true, but I am not comfortable with how threads are
integrated (i.e. it is not visible enough IMO). I wonder if there are
any troubles there. Those threads of course communicate through system
calls.

I might have fixed some of the problems with related to frames which
calls crashes, but there might be more.

Menus are rather often broken. I can see that in my patched version
because menus are more easily accessed there. I surely

And I wonder why Emacs sometimes seems to hang temporarily when frame
related operations are going on. Is something going wrong?

And the inability to stop Emacs sometimes is quite disturbing. What is
going on there? Why does not the GUI thread get resources? Or is it
given resources but a block occur in some other way? Is GUI messages
not checked when looping?


>>> This is even worse, because it's impossible to understand what this
>>> does without looking up the magical W32DEBPRINT macro.
>>
>>
>> I think it has some advantages, actually. It is easy to see and
>> distinguish from the normal code. It can be a null op in non-debugging
>> compilations.
>
> IMHO is horribly ugly. If you're going to have error checks, better to
> have them on the clear.


So both you and Eli think that way is ugly. I am a bit surprised (but
I accept it of course). This type of entering the checks is very
similar to what `assert' does in Emacs. Does that mean you think
`assert' is very ugly too or is there some difference I am missing?

Re: System calls without error checks in w32

[Juanma Barranquero]

On Mon, May 31, 2010 at 00:37, Lennart Borgman
<lennart.borgman@gmail.com> wrote:

> Yes, my intention was not to add them everywhere, only on interesting places.

It didn't seem so in your first message.

> On a quick look I got the impression that file calls are better
> handled than GUI system calls.

Likely because file calls are much more likely to fail.

> It happens to me with my patched Emacs because that is the one I am
> using. I only use unpatched Emacs for bug testing etc.

Why don't you add these checks to your patched Emacs and see what comes of it?

> That is surely true, but I am not comfortable with how threads are
> integrated (i.e. it is not visible enough IMO). I wonder if there are
> any troubles there. Those threads of course communicate through system
> calls.

Do you wonder where "there are any troubles there", or do you suspect
that there are troubles?

> I might have fixed some of the problems with related to frames which
> calls crashes, but there might be more.

Well, perhaps you've fixed them, but as I've said, your Emacs seem to
crash more often than most others ;-)

> And I wonder why Emacs sometimes seems to hang temporarily when frame
> related operations are going on. Is something going wrong?

You'd have to be a bit more specific...

> And the inability to stop Emacs sometimes is quite disturbing. What is
> going on there? Why does not the GUI thread get resources? Or is it
> given resources but a block occur in some other way? Is GUI messages
> not checked when looping?

Also here.

> This type of entering the checks is very
> similar to what `assert' does in Emacs. Does that mean you think
> `assert' is very ugly too or is there some difference I am missing?

No, I quite like asserts. The difference is than assert is

  assert (some test)

There's nothing to understand there. It passes, or it fails.
W32DEBPRINT has four arguments; at the very least, if would make more
sense to do

   W32DEBPRINT (this_function, ApiFunction (args) != bad_return_value)

or somesuch, and, at that point, you're just reinventing assert, which
seems pointless.

Anyway, my feeling is that it makes much more sense to check return
codes *to do something* about them in the case of failure, that just
to print a message which will be helpful or not, depending of how
frequent / how informative they are. So if you're really interested in
doing this, I'd suggest looking at specific unchecked syscalls and
trying to determine if something (could / need to) be done in case of
error. And then send patches ;-)

    Juanma

Re: System calls without error checks in w32

[Lennart Borgman]

On Mon, May 31, 2010 at 1:03 AM, Juanma Barranquero <lekktu@gmail.com> wrote:
> On Mon, May 31, 2010 at 00:37, Lennart Borgman
> <lennart.borgman@gmail.com> wrote:
>
>> Yes, my intention was not to add them everywhere, only on interesting places.
>
> It didn't seem so in your first message.


Maybe not, but perhaps even more so from the reply ;-)


>> On a quick look I got the impression that file calls are better
>> handled than GUI system calls.
>
> Likely because file calls are much more likely to fail.


That is not what I think. I think instead that the consequences of
failing file calls are more visible and therefor much easier to catch.

And it looks to me from the code that the knowledge of other system
calls is less or at least more problematic to handle. At least on the
w32 side. And it is not surprising since we are few here looking at
that code. That is also one reason for adding debug calls since it can
give more knowledge of what is going on in that code.


>> It happens to me with my patched Emacs because that is the one I am
>> using. I only use unpatched Emacs for bug testing etc.
>
> Why don't you add these checks to your patched Emacs and see what comes of it?


It takes me quite a lot of time to take care of all the patches. There
does not seem to be much interest or time to fix many of the issues
and then I have to keep them in my patched version. I am trying to
minimize that work.

Also it is much more trouble making a patch later if I have a lot of
small patches that are not in the upstream Emacs.

I have wasted quite a lot of time on this.

One reason is of course that I do not install my patches myself. Maybe
it is time to do that instead. Is there any easy way out of the
situation that I have my checkout from Launchpad. Can I somehow make
bzr aware of that this is really the same thing as on savannah and
upload from it? Or should I make a new checkout from savannah and use
that just for this?


>> That is surely true, but I am not comfortable with how threads are
>> integrated (i.e. it is not visible enough IMO). I wonder if there are
>> any troubles there. Those threads of course communicate through system
>> calls.
>
> Do you wonder where "there are any troubles there", or do you suspect
> that there are troubles?


Both ;-)

How does blocking interfere with that code? Are the system calls
always done in the right thread (or with correct thread
communication)?


>> I might have fixed some of the problems with related to frames which
>> calls crashes, but there might be more.
>
> Well, perhaps you've fixed them, but as I've said, your Emacs seem to
> crash more often than most others ;-)


I think I have given the reason for that. So far I have never been
able to identify a crash as something depending on my code. And
believe it or not that is what I try to investigate first (which makes
my quite a bit angry about having to keep them myself)... ;-)


>> And I wonder why Emacs sometimes seems to hang temporarily when frame
>> related operations are going on. Is something going wrong?
>
> You'd have to be a bit more specific...


I see this when creating a frame in a timer. Or perhaps when fetching
info from the web in a timer. I am not sure, but none of these should
block.

Of course there must not be system calls involved directly. It might
be "bad chaining", i.e. code that is unnecessarily waiting for system
calls to complete.


>> And the inability to stop Emacs sometimes is quite disturbing. What is
>> going on there? Why does not the GUI thread get resources? Or is it
>> given resources but a block occur in some other way? Is GUI messages
>> not checked when looping?
>
> Also here.


Run something in a timer. Try to stop it.


>> This type of entering the checks is very
>> similar to what `assert' does in Emacs. Does that mean you think
>> `assert' is very ugly too or is there some difference I am missing?
>
> No, I quite like asserts. The difference is than assert is
>
>  assert (some test)
>
> There's nothing to understand there. It passes, or it fails.
> W32DEBPRINT has four arguments; at the very least, if would make more
> sense to do
>
>   W32DEBPRINT (this_function, ApiFunction (args) != bad_return_value)
>
> or somesuch, and, at that point, you're just reinventing assert, which
> seems pointless.


Oh, the four args where just to make it more easy to understand... ;-)

Do you mean there is an assert that can do DebPrint? That would be very nice.


> Anyway, my feeling is that it makes much more sense to check return
> codes *to do something* about them in the case of failure, that just
> to print a message which will be helpful or not, depending of how
> frequent / how informative they are.


Things sometimes does not go totally wrong with system calls even if
something is wrong because the system will try to recover. Looking at
the messages might then give some information.

The problem as I have seen it (it was a long time since I did
something like this) is often that you get too much information so you
have to shut down the logging and only have it on for short tests.
(That is one reason to keep the debug printing statements, but
commented out.)


> So if you're really interested in
> doing this, I'd suggest looking at specific unchecked syscalls and
> trying to determine if something (could / need to) be done in case of
> error. And then send patches ;-)


Sending patches is much easier if you do not have to remove a lot of
things from the diffs.

Re: System calls without error checks in w32

[Juanma Barranquero]

On Mon, May 31, 2010 at 01:28, Lennart Borgman
<lennart.borgman@gmail.com> wrote:

> That is not what I think. I think instead that the consequences of
> failing file calls are more visible and therefor much easier to catch.

They wouldn't be catched if they didn't happen. If other system calls
are causing as many errors as you suggest, it is strange the sparsity
of bug reports about them.

> And it looks to me from the code that the knowledge of other system
> calls is less or at least more problematic to handle.

I cannot parse this, sorry.

> At least on the
> w32 side. And it is not surprising since we are few here looking at
> that code. That is also one reason for adding debug calls since it can
> give more knowledge of what is going on in that code.

Usually, bugs have a tendendy to show up even if you're not looking for them...

> It takes me quite a lot of time to take care of all the patches. There
> does not seem to be much interest or time to fix many of the issues
> and then I have to keep them in my patched version.

Here we go again. "Does not seem to be much interest or time" sort of
implies that you've done the work of sending patches and they've been
summarily rejected. From my recollection, you seem to lose interest
quite fast as soon as you fail to immediately convince that they are
worthwhile and correct (sorry if this seems an ad hominem; it's not.
I'm just describing how I remember past interactions, particularly
with respect to emacsclient).

> Also it is much more trouble making a patch later if I have a lot of
> small patches that are not in the upstream Emacs.

That's what branches are for :-)

> One reason is of course that I do not install my patches myself. Maybe
> it is time to do that instead. Is there any easy way out of the
> situation that I have my checkout from Launchpad. Can I somehow make
> bzr aware of that this is really the same thing as on savannah and
> upload from it? Or should I make a new checkout from savannah and use
> that just for this?

The latter. But I'd advise against installing these kind of changes
without discussing them first with Eli and Jason, which shoulder most
of the w32 effort.

> Both ;-)

Well, if you know there are troubles, you can patch your Emacs and debug them...

> How does blocking interfere with that code? Are the system calls
> always done in the right thread (or with correct thread
> communication)?

Again: for specific problems, file bug reports. Perhaps there are
really some missing checks on some syscalls, but the fix for your
troubles is likely to be more than just calling DebPrint.

> I think I have given the reason for that. So far I have never been
> able to identify a crash as something depending on my code.

Have you been able to identify them as something depending on the
trunk code? If so, did you file reports for them?

> I see this when creating a frame in a timer. Or perhaps when fetching
> info from the web in a timer. I am not sure, but none of these should
> block.
> Of course there must not be system calls involved directly. It might
> be "bad chaining", i.e. code that is unnecessarily waiting for system
> calls to complete.

Specific examples would help in debugging this.

> Oh, the four args where just to make it more easy to understand... ;-)

I think you mean s/more/less/.

> Do you mean there is an assert that can do DebPrint? That would be very nice.

No, I mean that if the syscalls are failing in such a way that there's
no fix to be done after the fact, triggering an assert is the Right
Thing To Do (that will help detect the problem).

> Things sometimes does not go totally wrong with system calls even if
> something is wrong because the system will try to recover. Looking at
> the messages might then give some information.

That is not an argument *for* printing information, is an argument for
checking the return code more thoroughly and doing the right thing.

> Sending patches is much easier if you do not have to remove a lot of
> things from the diffs.

Do you really have so many changes in the w32*.c files that doing a
diff is a problem? Perhaps it's time you start calling EmacsW32 a fork
;-)

    Juanma

Re: System calls without error checks in w32

[Lennart Borgman]

On Mon, May 31, 2010 at 2:10 AM, Juanma Barranquero <lekktu@gmail.com> wrote:
> On Mon, May 31, 2010 at 01:28, Lennart Borgman
> <lennart.borgman@gmail.com> wrote:
>
>> That is not what I think. I think instead that the consequences of
>> failing file calls are more visible and therefor much easier to catch.
>
> They wouldn't be catched if they didn't happen. If other system calls
> are causing as many errors as you suggest, it is strange the sparsity
> of bug reports about them.


I don't think so. It is just much more difficult to track some of
these. For example I know Drew recently wrote about problems when
creating new frames. It is quite hard to both understand what is
happening and what is a bug and what is not. Is there just a bad
timing? A missing hook to proceed the function etc (you may have to
wait until the frame creation is finished)?



>> And it looks to me from the code that the knowledge of other system
>> calls is less or at least more problematic to handle.
>
> I cannot parse this, sorry.


I just mean that the normal C code (i.e. not related to system calls)
seems better checked on w32.


> Usually, bugs have a tendendy to show up even if you're not looking for them...


Maybe you are missing my point. The system tries to stay stable. There
are timing issues etc.


>> It takes me quite a lot of time to take care of all the patches. There
>> does not seem to be much interest or time to fix many of the issues
>> and then I have to keep them in my patched version.
>
> Here we go again. "Does not seem to be much interest or time" sort of
> implies that you've done the work of sending patches and they've been
> summarily rejected. From my recollection, you seem to lose interest
> quite fast as soon as you fail to immediately convince that they are
> worthwhile and correct (sorry if this seems an ad hominem; it's not.
> I'm just describing how I remember past interactions, particularly
> with respect to emacsclient).


No. I would rather say I just don't have time then.

When I entered this list I went into a long discussion about the
problems with printing on the w32 side. I actually felt I wasted a lot
of time when I told that the code was in error. Since then I have
thought it is less waste of time to keep the patches in my patched
version. Now and then I ask if things have changed. If they have not I
just skip it, but with some regret about the extra trouble and lost
work.


>> Also it is much more trouble making a patch later if I have a lot of
>> small patches that are not in the upstream Emacs.
>
> That's what branches are for :-)


Yes, it seems easier, but I will have to create quite a few branches
for some old patches. And it takes time.


>> One reason is of course that I do not install my patches myself. Maybe
>> it is time to do that instead. Is there any easy way out of the
>> situation that I have my checkout from Launchpad. Can I somehow make
>> bzr aware of that this is really the same thing as on savannah and
>> upload from it? Or should I make a new checkout from savannah and use
>> that just for this?
>
> The latter. But I'd advise against installing these kind of changes
> without discussing them first with Eli and Jason, which shoulder most
> of the w32 effort.


Thanks. Yes, of course.


>> Both ;-)
>
> Well, if you know there are troubles, you can patch your Emacs and debug them...


See above.


>> How does blocking interfere with that code? Are the system calls
>> always done in the right thread (or with correct thread
>> communication)?
>
> Again: for specific problems, file bug reports.


I have done that many times (or sent reports to the list before) but
these kind of errors are not easy to track down. And since I have some
patches (that correct some bugs, but also makes other bugs more
visible) they tend to be dismissed.

It is kind of a locked situation.


> Perhaps there are
> really some missing checks on some syscalls, but the fix for your
> troubles is likely to be more than just calling DebPrint.


There are a lot of them as I said before.


>> I think I have given the reason for that. So far I have never been
>> able to identify a crash as something depending on my code.
>
> Have you been able to identify them as something depending on the
> trunk code? If so, did you file reports for them?


It seems much more trouble reporting the problems than fixing them.
Maybe I should try to make a "system calls fix branch" from savannah
and pick them one by one when I have time.


>> I see this when creating a frame in a timer. Or perhaps when fetching
>> info from the web in a timer. I am not sure, but none of these should
>> block.
>> Of course there must not be system calls involved directly. It might
>> be "bad chaining", i.e. code that is unnecessarily waiting for system
>> calls to complete.
>
> Specific examples would help in debugging this.


I have an example where I create a frame contact a web page in
pause.el. Is that specific enough? (You have to run my patched code to
see it since I can't do the frame manipulation I need without that.)


>> Oh, the four args where just to make it more easy to understand... ;-)
>
> I think you mean s/more/less/.


No ;-)


>> Do you mean there is an assert that can do DebPrint? That would be very nice.
>
> No, I mean that if the syscalls are failing in such a way that there's
> no fix to be done after the fact, triggering an assert is the Right
> Thing To Do (that will help detect the problem).


I don't think so. You should most often not crash Emacs because of bad
system call return status. You should take a relevant action.


>> Things sometimes does not go totally wrong with system calls even if
>> something is wrong because the system will try to recover. Looking at
>> the messages might then give some information.
>
> That is not an argument *for* printing information, is an argument for
> checking the return code more thoroughly and doing the right thing.


Of course, but you may be interested in information about what happened too.


>> Sending patches is much easier if you do not have to remove a lot of
>> things from the diffs.
>
> Do you really have so many changes in the w32*.c files that doing a
> diff is a problem? Perhaps it's time you start calling EmacsW32 a fork
> ;-)


Perhaps not, but sometimes it is a bit scary when merging as it was
recently when I had some changes to local/global variables. A variant
of those are now in Emacs so I can skip worrying about that any more.

And the changes that allows better menu handling are also a bit
scaring when merging. They are still only in my code.


>    Juanma

Re: System calls without error checks in w32

[Juanma Barranquero]

On Mon, May 31, 2010 at 02:58, Lennart Borgman
<lennart.borgman@gmail.com> wrote:

> I don't think so. It is just much more difficult to track some of
> these. For example I know Drew recently wrote about problems when
> creating new frames. It is quite hard to both understand what is
> happening and what is a bug and what is not.

Well, I'd say that should be the first step...

> I just mean that the normal C code (i.e. not related to system calls)
> seems better checked on w32.

Not sure what do you mean with "better checked". Obviously it is
easier to know what one of our own C functions is doing that what a
Windows system call does, so it's less likely that a return code from
a C function is ignored.

> Maybe you are missing my point. The system tries to stay stable. There
> are timing issues etc.

Maybe. Sometimes it seems like you assume your interlocutor knows it
all about the bugs you encounter; I tend to find your descriptions
vague and hard to follow (not necessarily your fault, of course).

> When I entered this list I went into a long discussion about the
> problems with printing on the w32 side. I actually felt I wasted a lot
> of time when I told that the code was in error. Since then I have
> thought it is less waste of time to keep the patches in my patched
> version. Now and then I ask if things have changed. If they have not I
> just skip it, but with some regret about the extra trouble and lost
> work.

I see no practical difference between my take of the situation
(regarding your patches) and yours.

> I have done that many times (or sent reports to the list before) but
> these kind of errors are not easy to track down. And since I have some
> patches (that correct some bugs, but also makes other bugs more
> visible) they tend to be dismissed.

I don't think so. They aren't dismissed because of your patches, but
because they are (at least, some that I've followed) very hard to
reproduce in standard Emacs.

> It seems much more trouble reporting the problems than fixing them.
> Maybe I should try to make a "system calls fix branch" from savannah
> and pick them one by one when I have time.

Perhaps it'd be a good idea.

> I have an example where I create a frame contact a web page in
> pause.el. Is that specific enough? (You have to run my patched code to
> see it since I can't do the frame manipulation I need without that.)

Then, why are you sure the problem isn't in your code? Surely if it is
a bug in Emacs there will be a way to show it with the trunk code?

> No ;-)

Hmm... allow me to disagree ;-)

> I don't think so. You should most often not crash Emacs because of bad
> system call return status. You should take a relevant action.

That's explicit in my comment. If you can take relevant action, do it
(not DebPrint, but a really relevant action). If you cannot, and the
bug is real, then abort.

> Of course, but you may be interested in information about what happened too.

While debugging. Not as a general principle. I don't want to run Emacs
under GDB for a totally unrelated reason and have a lot of noise. I
want that "noise", if it is relevant, to be acted upon. So, instead of
just adding lots of DebPrints, it would be more sensible to add them
locally for debugging, find what's happening (surely if you have so
many troubles, the error output will be abundant) and either fix it or
file a bug report for it.

> And the changes that allows better menu handling are also a bit
> scaring when merging. They are still only in my code.

Perhaps the latter is consequence of the former.

    Juanma

Re: System calls without error checks in w32

[Lennart Borgman]

On Mon, May 31, 2010 at 4:02 AM, Juanma Barranquero <lekktu@gmail.com> wrote:
> On Mon, May 31, 2010 at 02:58, Lennart Borgman
> <lennart.borgman@gmail.com> wrote:
>
>> I don't think so. It is just much more difficult to track some of
>> these. For example I know Drew recently wrote about problems when
>> creating new frames. It is quite hard to both understand what is
>> happening and what is a bug and what is not.
>
> Well, I'd say that should be the first step...

Exactly. That is why debug output might be helpful.

>> I just mean that the normal C code (i.e. not related to system calls)
>> seems better checked on w32.
>
> Not sure what do you mean with "better checked". Obviously it is
> easier to know what one of our own C functions is doing that what a
> Windows system call does, so it's less likely that a return code from
> a C function is ignored.

Yes, but isn't it a really good reason to always check the return code
from system calls?

> I see no practical difference between my take of the situation
> (regarding your patches) and yours.

Maybe it is just me then. But I have thrown away a good deal of them
just to get less work (even if some of them were bug fixes).

>> I have an example where I create a frame contact a web page in
>> pause.el. Is that specific enough? (You have to run my patched code to
>> see it since I can't do the frame manipulation I need without that.)
>
> Then, why are you sure the problem isn't in your code? Surely if it is
> a bug in Emacs there will be a way to show it with the trunk code?

Because they are not related to that part of the code.

>> No ;-)
>
> Hmm... allow me to disagree ;-)

What can I do? ;-)

> While debugging. Not as a general principle. I don't want to run Emacs
> under GDB for a totally unrelated reason and have a lot of noise.

Yes, of course. But keeping a commented out one line debug output call
might be quit useful in some places.

>> And the changes that allows better menu handling are also a bit
>> scaring when merging. They are still only in my code.
>
> Perhaps the latter is consequence of the former.

Yes, I know.

Re: System calls without error checks in w32

[Juanma Barranquero]

On Mon, May 31, 2010 at 04:36, Lennart Borgman
<lennart.borgman@gmail.com> wrote:

> Yes, but isn't it a really good reason to always check the return code
> from system calls?

Yes. Check and *do something* about it.

>> Then, why are you sure the problem isn't in your code? Surely if it is
>> a bug in Emacs there will be a way to show it with the trunk code?
>
> Because they are not related to that part of the code.

That does not answer the second question.

> Yes, of course. But keeping a commented out one line debug output call
> might be quit useful in some places.

The trick is to find these (supposedly few) "some places".

    Juanma

Re: System calls without error checks in w32

[Eli Zaretskii]

> From: Lennart Borgman <lennart.borgman@gmail.com>
> Date: Mon, 31 May 2010 00:37:26 +0200
> Cc: Eli Zaretskii <eliz@gnu.org>, emacs-devel@gnu.org
> 
> On a quick look I got the impression that file calls are better
> handled than GUI system calls.

That's because when it happens with files, it's quite clear what to
do.  But what do you do when you try to display a menu, and the API
returns an error?

> So both you and Eli think that way is ugly.

It obfuscates the source code, that's the main problem.

Re: System calls without error checks in w32

[grischka]

 > ... For example I know Drew recently wrote about problems when
 > creating new frames. It is quite hard to both understand what is
 > happening and what is a bug and what is not. Is there just a bad
 > timing? A missing hook to proceed the function etc (you may have to
 > wait until the frame creation is finished)?

Typically such problems have to do with the two-thread approach in the
"w32" code, which again has to do with emacs unfortunate event system
(aka. command_loop) which is not really compatible with GUI backends
(w32, gtk).

Basically:  Replace the command_loop by callbacks, then get rid of the
w32-GUI-thread, then watch how the problems disappeared.  Magically ;-)

--- grischka

Re: System calls without error checks in w32

[Jason Rumney]

grischka <grishka@gmx.de> writes:

> Basically:  Replace the command_loop by callbacks, then get rid of the
> w32-GUI-thread, then watch how the problems disappeared.  Magically ;-)

Some problems may disappear, but they will be replaced by many more
serious problems; like windows that can't be moved while Lisp code is
busy running.

Re: System calls without error checks in w32

[grischka]

Jason Rumney wrote:
> grischka <grishka@gmx.de> writes:
> 
>> Basically:  Replace the command_loop by callbacks, then get rid of the
>> w32-GUI-thread, then watch how the problems disappeared.  Magically ;-)
> 
> Some problems may disappear, but they will be replaced by many more
> serious problems; like windows that can't be moved while Lisp code is
> busy running.

XEmacs uses the same thread for lisp and gui and doesn't have any
such problem.

--- grischka

Re: System calls without error checks in w32

[Stephen J. Turnbull]

grischka writes:

 > XEmacs uses the same thread for lisp and gui and doesn't have any
 > such problem.

XEmacs has such a hairy event loop that it is no longer supported by
X.org.  You may not want to go there.

http://bugs.freedesktop.org/show_bug.cgi?id=13471
https://trac.macports.org/ticket/18491
http://bugs.freedesktop.org/show_bug.cgi?id=20048

Re: System calls without error checks in w32

[Lennart Borgman]

On Mon, Jun 7, 2010 at 4:54 PM, Stephen J. Turnbull <stephen@xemacs.org> wrote:
> grischka writes:
>
>  > XEmacs uses the same thread for lisp and gui and doesn't have any
>  > such problem.
>
> XEmacs has such a hairy event loop that it is no longer supported by
> X.org.  You may not want to go there.
>
> http://bugs.freedesktop.org/show_bug.cgi?id=13471
> https://trac.macports.org/ticket/18491
> http://bugs.freedesktop.org/show_bug.cgi?id=20048

;-)

I guess the idea of having two event loops/thread is not in itself
bad. If the gui thread is never blocked it can be used to convey
status information at least to the user. (You can put these in
temporary windows inside the frame for example.)

But sometimes I wonder wheter all windows really runs in the gui
threads. And why does not C-g break all looping, even those in timers
(like fontification)? And is it the same trouble on other platfoms or
is it just the w32 port that has this?

And moreover I wonder wether all messages really reach their
destinations or if something is blocking them in certain situations.
(Again, why is not C-g handled in the case above?)

And what is blocking Emacs during the time when a new frame is
created? Why is not that done asynchronously?

Re: System calls without error checks in w32

[grischka]

Lennart Borgman wrote:
> I guess the idea of having two event loops/thread is not in itself
> bad. If the gui thread is never blocked it can be used to convey
> status information at least to the user. (You can put these in
> temporary windows inside the frame for example.)

Since this is Emacs you'd probably want to format your status info
in Lisp.  Bye nice idea and welcome to thread hell.

> But sometimes I wonder wheter all windows really runs in the gui
> threads. And why does not C-g break all looping, even those in timers
> (like fontification)? And is it the same trouble on other platfoms or
> is it just the w32 port that has this?
>
> And moreover I wonder wether all messages really reach their
> destinations or if something is blocking them in certain situations.
> (Again, why is not C-g handled in the case above?)
> 
> And what is blocking Emacs during the time when a new frame is
> created? Why is not that done asynchronously?
> 

I guess there are some people wondering sometimes about something.
Thread bugs typically show up sometimes.  And typically you'll find
nobody who wants to look at them.

--- grischka

Re: System calls without error checks in w32

[Lennart Borgman]

On Mon, Jun 7, 2010 at 7:21 PM, grischka <grishka@gmx.de> wrote:
> Lennart Borgman wrote:
>>
>> I guess the idea of having two event loops/thread is not in itself
>> bad. If the gui thread is never blocked it can be used to convey
>> status information at least to the user. (You can put these in
>> temporary windows inside the frame for example.)
>
> Since this is Emacs you'd probably want to format your status info
> in Lisp.  Bye nice idea and welcome to thread hell.


I see no reason to format it in Lisp. Or rather my idea is that the
status information is formated by the lisp thread in advance and sent
to the gui thread in a non-lisp format. Then the gui thread can offer
that information without contactinog the lisp thread.

Displaying that information is just a matter of formatting a native
w32 window with it.

Similar things can of course be done on other platforms, but this part
of the code is necessary platform specif (at least in the core parts
of it).


> I guess there are some people wondering sometimes about something.
> Thread bugs typically show up sometimes.  And typically you'll find
> nobody who wants to look at them.


Looking at this kind of bugs is hard and requires that you have a good
enough picture of the message queue system. And just guessing what to
look at is difficult.

Re: System calls without error checks in w32

[grischka]

Lennart Borgman wrote:
> On Mon, Jun 7, 2010 at 7:21 PM, grischka <grishka@gmx.de> wrote:
>> Lennart Borgman wrote:
>>> I guess the idea of having two event loops/thread is not in itself
>>> bad. If the gui thread is never blocked it can be used to convey
>>> status information at least to the user. (You can put these in
>>> temporary windows inside the frame for example.)
> 
>> Since this is Emacs you'd probably want to format your status info
>> in Lisp.  Bye nice idea and welcome to thread hell.
> 
> I see no reason to format it in Lisp. Or rather my idea is that the
> status information is formated by the lisp thread in advance and sent
> to the gui thread in a non-lisp format. Then the gui thread can offer
> that information without contactinog the lisp thread.

But then what is the benefit from that additional effort of preparing
and sending a "non-lisp" representation forth and back, except to satisfy
the requirements of having two threads?  Have only one thread and you
can format and display the info in one go.  Simple, reliable, conclusive.

--- grischka

Re: System calls without error checks in w32

[Lennart Borgman]

On Mon, Jun 7, 2010 at 8:54 PM, grischka <grishka@gmx.de> wrote:
> Lennart Borgman wrote:
>>
>> On Mon, Jun 7, 2010 at 7:21 PM, grischka <grishka@gmx.de> wrote:
>>>
>>> Lennart Borgman wrote:
>>>>
>>>> I guess the idea of having two event loops/thread is not in itself
>>>> bad. If the gui thread is never blocked it can be used to convey
>>>> status information at least to the user. (You can put these in
>>>> temporary windows inside the frame for example.)
>>
>>> Since this is Emacs you'd probably want to format your status info
>>> in Lisp.  Bye nice idea and welcome to thread hell.
>>
>> I see no reason to format it in Lisp. Or rather my idea is that the
>> status information is formated by the lisp thread in advance and sent
>> to the gui thread in a non-lisp format. Then the gui thread can offer
>> that information without contactinog the lisp thread.
>
> But then what is the benefit from that additional effort of preparing
> and sending a "non-lisp" representation forth and back, except to satisfy
> the requirements of having two threads?  Have only one thread and you
> can format and display the info in one go.  Simple, reliable, conclusive.


I suppose the rational for having two threads is that basic gui
operations should not be stalled by lisp computations. I do not know
how it is actually implemented now in Emacs but with two threads you
can post messages between them to coordinate them without interrupting
the gui handling badly. Here is an example:

- the user resize the frame during a lisp computation. The resizing
happens then entirely in the gui thread.

- When resizing is finished the gui thread can send a message telling
that to the lisp queue.

- The lisp queue can then handle this when it has time to do that and
ask the system for the new frame size.

- The gui thread can also send resizing messages during resizing which
the lisp thread can safely ignore if it is occupied since the
information is available later.
The gui thread can send a message to the lisp thread when resizing is
finished and tell that resizing has been done. The

Re: System calls without error checks in w32

[Lennart Borgman]

> - The gui thread can also send resizing messages during resizing which
> the lisp thread can safely ignore if it is occupied since the
> information is available later.
> The gui thread can send a message to the lisp thread when resizing is
> finished and tell that resizing has been done. The

A shit, this is how firefox sometimes handle input when GDI resources
are exhausted. You have to guess the end part of the message for now
... ;-)

Re: System calls without error checks in w32

[Stefan Monnier]

> threads. And why does not C-g break all looping, even those in timers
> (like fontification)?

That is simply because timer code is normally run in a way that's
completely hidden from the user, so if the user happens to hit C-g while
a timer is running, it's probably not because he wants to interrupt the
timer, but rather because he wants to interrupt the thing he's
currently doing.

Timers (and fontification, post-command-hook, ...) should run quickly,
or otherwise re-enable interrupts using with-local-quit.


        Stefan

Re: System calls without error checks in w32

[Lennart Borgman]

On Mon, Jun 7, 2010 at 9:56 PM, Stefan Monnier <monnier@iro.umontreal.ca> wrote:
>> threads. And why does not C-g break all looping, even those in timers
>> (like fontification)?
>
> That is simply because timer code is normally run in a way that's
> completely hidden from the user, so if the user happens to hit C-g while
> a timer is running, it's probably not because he wants to interrupt the
> timer, but rather because he wants to interrupt the thing he's
> currently doing.

Hm, I always hit C-g to interrupt something that Emacs is doing... ;-)

How about some convention: Three C-g within three seconds will stop
running timers too?

> Timers (and fontification, post-command-hook, ...) should run quickly,
> or otherwise re-enable interrupts using with-local-quit.

Thanks, I see, that is an idea. I had forgotten about it.