Project

General

Profile

Bug #798

SigTerm during file I/O causes gmx_error

Added by Roland Schulz over 7 years ago. Updated about 7 years ago.

Status:
Closed
Priority:
High
Assignee:
Category:
-
Target version:
Affected version - extra info:
Affected version:
Difficulty:
uncategorized
Close

Description

The error handler is setup in signal_handler_install using the "signal" function. If a system call is running (e.g. I/O) while the function is running the system call fails with EINTR. One can check everywhere for EINTR and rerun the system call. But much easier is to set the signal behavior to SA_RESTART.

This flag is only available with the function "sigaction" and not with "signal". "signal" doesn't specify whether a syscall is interrupted or not (it is for e.g. Linux). Thus I suggest that in signal_handler_install function "signal" is replaced by sigaction and SA_RESTART is set.

For systems without sigaction (e.g. Windows), we could continue to use "signal".

Associated revisions

Revision bf2fcd92 (diff)
Added by Roland Schulz about 7 years ago

Restart IO operations when interrupter by signal

Fixes #798

Change-Id: I762412abf8138bbbe3d386d015e6af3522bd5235

Revision 6cb62f77 (diff)
Added by Roland Schulz about 7 years ago

Fix for sigaction

Commit "Restart IO operations when interrupter by signal"
didn't actually activate sigaction because config.h.cmakein
change was missing.

Fixes #798

Change-Id: Id5c7e3344688c32ec4c51d84ccbc66658ed928f9

History

#1 Updated by Rossen Apostolov over 7 years ago

  • Assignee set to Sander Pronk

#2 Updated by Roland Schulz over 7 years ago

  • Target version set to 4.5.5

#3 Updated by Rossen Apostolov over 7 years ago

  • Target version changed from 4.5.5 to 4.5.6

#4 Updated by Rossen Apostolov about 7 years ago

Shall this get fixed for 4.5.6?

#5 Updated by Roland Schulz about 7 years ago

I think so. I'm happy to make the proposed change myself - is very easy. I didn't do this initially because I wanted get feedback on whether the solution is correct. Should I go ahead and push a change to gerrit to get feedback there?

#6 Updated by Rossen Apostolov about 7 years ago

Sure, please do that.

Sander, what do you think about this patch?

#7 Updated by Roland Schulz about 7 years ago

Sander, can you let me know what you think about the idea as described? Or do you want me to upload first the patch and then comment there?

#8 Updated by Rossen Apostolov about 7 years ago

  • Priority changed from Normal to High

Could you patch it for 4.5.6?

#9 Updated by Roland Schulz about 7 years ago

  • Target version changed from 4.5.6 to 4.6

See https://gerrit.gromacs.org/#/c/719/ for reason to change target.

#10 Updated by Rossen Apostolov about 7 years ago

OK, that's good

#11 Updated by Rossen Apostolov about 7 years ago

  • Status changed from New to Closed

Closing, the patch is approved and just needs to be merged.

Also available in: Atom PDF