818106 - ScopedClose doesn't deal with EINTR

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Dave Hylands [:dhylands]]

close can fail with errno == EINTR, in which case the close call needs to be made again.

ScopedClose just calls close and doen't check the return code, assuming that the close succeeded. If the close returns EINTR, then the close may not have actually happened.

Comment 1

[Josh Matthews [:jdm]]

For whomever takes this one, see http://hg.mozilla.org/mozilla-central/annotate/287a7d7cf7f0/xpcom/glue/FileUtils.h#l54

Comment 2

[gautam4you]

Just changing the close(fd); to while(close(fd)==EINTR); is enough ? 
Or should there be any maximum retries kind of logic ? 
Or should it wait for some time before retrying again ? 
Please let me know the details

Comment 3

[Dave Hylands [:dhylands]]

The actual logic should be:

while ((close(fd) == -1) && (errno == EINTR)) {
    ;
}

You shouldn't need any maximum retries logic. This is essentially the same logic found in the
TEMP_FAILURE_RETRY macro from /usr/include/unistd.h, but that macro is glibc specific, so it may not be portable across all of the platforms that we support.

Comment 4

[[:badanomaly]]

Attachment: Patch 1

Comment 5

[[:badanomaly]]

I have attached the patch. Please Review it.

Comment 6

[[:badanomaly]]

Attachment: Patch 2

Comment 7

[[:badanomaly]]

Attachment: Patch 3

Comment 8

[Dave Hylands [:dhylands]]

Comment on attachment 697219 [details] [diff] [review]
Patch 3

Review of attachment 697219 [details] [diff] [review]:
-----------------------------------------------------------------

::: xpcom/glue/FileUtils.h
@@ +49,5 @@
>    static void release(type fd) {
> +    if (fd != -1){
> +      do{
> +        close(fd);
> +      }while (errno == EINTR);

This won't work. errno isn't set if close returns successfully. You should only test errno if close returns -1, or if you explicitly set errno to 0 prior to calling close.

Comment 9

[Dave Hylands [:dhylands]]

Also:

nit: You should have have a space bewteen } and while and also between ) and {

Comment 10

[[:badanomaly]]

Attachment: Patch 4

Comment 11

[[:badanomaly]]

How do I get this bug assigned to me. I have attached the patch still its showing assigned to: nobody ?

Comment 12

[Dave Hylands [:dhylands]]

In bugzilla, click on the "take" link next to the "Assigned To" and then click on Save.

Comment 13

[[:badanomaly]]

(In reply to Dave Hylands [:dhylands] from comment #12)
> In bugzilla, click on the "take" link next to the "Assigned To" and then
> click on Save.

clicking on the link creates a new email addressed to [e-mail] .
Am i supposed to send an email ?

Comment 14

[Dave Hylands [:dhylands]]

Are you logged in?
There should be 2 links, one says edit, and one says take.

Comment 15

[Dave Hylands [:dhylands]]

Oops - I wound up assigning it to you. I was playing with logging in and out, and wound up assigning it to you.

So I'll assign it back to nobody.

Comment 16

[Dave Hylands [:dhylands]]

If you click on edit, it will shows who its currently assigned to, and you can change it (if you have permission). You should be able to change it to your email address.

Clicking on take should fill in your email address (so this should be easier than clicking on edit)

Comment 17

[Dave Hylands [:dhylands]]

Are you on IRC? It's probably easier to have a discussion there, rather than in the bug...

Comment 18

[Dave Hylands [:dhylands]]

Try:
https://tbpl.mozilla.org/?tree=Try&rev=cfc2657ae40c

Comment 19

[[:badanomaly]]

Attachment: Fix: ScopedClose now considers errno EINTR.

Comment 20

[Dave Hylands [:dhylands]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b0967de80fb7

Comment 21

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/b0967de80fb7

Comment 23

[Justin Lebar (not reading bugmail)]

> If you click on edit

Note that people new to bugzilla don't initially have permission to edit many of a bug's metadata fields.  You can go on IRC and ask someone to give X e-mail address "editbugs" permission.

Comment 24

[Dave Hylands [:dhylands]]

(In reply to Justin Lebar [:jlebar] from comment #23)
> > If you click on edit
> 
> Note that people new to bugzilla don't initially have permission to edit
> many of a bug's metadata fields.  You can go on IRC and ask someone to give
> X e-mail address "editbugs" permission.

Yeah - we eventually figured that out, and now that I'm aware that's one of the things I look for. Apparently, the proper procedure for getting editbugs is to follow this:
https://bugzilla.mozilla.org/page.cgi?id=get_permissions.html

and this page is generally a good page to read when first learning about bugzilla:
https://developer.mozilla.org/en/docs/What_to_do_and_what_not_to_do_in_Bugzilla