weixin_39954464
weixin_39954464
2020-12-29 19:29

Signal handlers call unsafe signal functions (race conditions may result in undefined behavior)

Great project. I noticed the following while browsing the code: The signal handler is not async signal safe (for example printf and fflush are called during the signal handler); this is a race condition and may result in the simulator crashing, generating incorrect output, or deadlocking when the window is resized.

http://man7.org/linux/man-pages/man7/signal.7.html "If a signal interrupts the execution of an unsafe function, and handler calls an unsafe function, then the behavior of the program is undefined."

 C
void sigWinChCatcher(int signum) {
    redrawScreen();
}
void redrawScreen() {
    updateConsoleSize();
    clearScreen();
    drawScreen();
    fflush(stdout);
}

Some suggestions: If the main event loop sleep() will be interrupted and return early so is it necessary to redraw the screen - why not delegate it back to the main thread? write(2) is async signal safe. Let the main loop know that a redraw is required after sleep using a simple boolean flag.

Similar arguments apply to the SIGINT handler. exit() is not async-signal-safe; a simple 'pleaseexit' boolean flag may be better implementation.

该提问来源于开源项目:gto76/comp-cpp

  • 点赞
  • 写回答
  • 关注问题
  • 收藏
  • 复制链接分享
  • 邀请回答

11条回答

  • weixin_39583029 weixin_39583029 4月前

    Thanks, I'll take some time to refresh the system programming knowledge, and think about suggestions and come back to you... For now all I can say is that I hoped that this C code (environment.c and output.c) could be as reusable as possible (kind of a poor man's curses). It is originally from my other project. What I changed is that drawScreen function is passed from main with setOutput function at initialization, and I hoped that could be all the interaction between the parts...

    点赞 评论 复制链接分享
  • weixin_39583029 weixin_39583029 4月前

    How about just setting a flag when redrawScreen is in progress? Like:

     c
    int catcherInAction = 0;
    
    // Fires when window size changes
    void sigWinChCatcher(int signum) {
        if (catcherInAction) {
            return;
        }
        catcherInAction = 1;
        redrawScreen();
        catcherInAction = 0;
    }
    
    点赞 评论 复制链接分享
  • weixin_39954464 weixin_39954464 4月前

    On 6/19/15 10:10 AM, Jure Šorn wrote:

    How about just setting a flag when redrawScreen is in progress?

    that would be an insufficient hack (but would likely reduce but not eliminate the chances of bad-things-TM); calling most library or system calls during a handler results in 'undefined behavior'

    Better: handle all redrawing in one place (your main thread) and then inform the main thread that a redraw is immediately required. You can check this boolean flag upon a return from sleep() ... and go back to sleep again if sleep was interrupted.

    (sleep will return early - you check the return value and errno == EINTR)

    — Reply to this email directly or view it on GitHub https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_gto76_comp-2Dcpp_issues_8-23issuecomment-2D113544115&d=AwMCaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=DvT4PEagC1_uJoK7XYcVznCf9vBnV8AqDdM20MaTLvM&m=exqnw0gEy1SZvWI3R188he1PG-4i6KsVEDDwBTczqkE&s=eyfpIbfDQHPYYxcZJMA7QUnJEH82nEE_qtttcsr-neg&e=.

    点赞 评论 复制链接分享
  • weixin_39583029 weixin_39583029 4月前

    Thing is that most waiting happens at:

     c
    char c = getc(stdin);
    

    and I don't know if you can interrupt getc...

    点赞 评论 复制链接分享
  • weixin_39954464 weixin_39954464 4月前

    On 6/19/15 1:03 PM, Jure Šorn wrote:

    Thing is that most waiting happens at:

    char c = getc(stdin);

    and I don't know if you can interrupt |getc|...

    — Reply to this email directly or view it on GitHub https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_gto76_comp-2Dcpp_issues_8-23issuecomment-2D113592487&d=AwMCaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=DvT4PEagC1_uJoK7XYcVznCf9vBnV8AqDdM20MaTLvM&m=VWP9prfWRV75sIm_xApDYti5PF8r90tIyZun0SEfkNk&s=BanRwxl8a_4i_QhZ1839T7UB3jw6nqaDrgJeyAz2kt0&e=.

    You can use the lower level read(2) which will return when interrupted

    ssize_t read(int fd, void *buf, size_t count); http://linux.die.net/man/2/read

    e.g. char c=0; errno=0; ssize_t num = read(0, &c, 1);

    if(num == -1 && errno == EINTR) { /* Interrupted by signal)

    点赞 评论 复制链接分享
  • weixin_39583029 weixin_39583029 4月前

    Thanks, will get on it...

    点赞 评论 复制链接分享
  • weixin_39583029 weixin_39583029 4月前

    I think both signal handlers are now async-signal-safe.

    (Only introduced change in programs behavior is that when computer is running, it doesn't automatically redraw screen on window size change, but waits for another cycle. But since cycles last a third of a second this actually makes a window resize smoother)

    点赞 评论 复制链接分享
  • weixin_39954464 weixin_39954464 4月前

    yes - if I read your code correctly, both signal handlers just set a flag, so yes that is certainly async signal safe.

    On 6/21/15 1:50 PM, Jure Šorn wrote:

    I think both signal handlers are now async-signal-safe.

    (Only introduced change in programs behavior is that when computer is running, it doesn't automatically redraw screen on window size change, but waits for another cycle. But since cycles last a third of a second this actually makes a window resize smoother)

    — Reply to this email directly or view it on GitHub https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_gto76_comp-2Dcpp_issues_8-23issuecomment-2D113941236&d=AwMCaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=DvT4PEagC1_uJoK7XYcVznCf9vBnV8AqDdM20MaTLvM&m=SA7dQcTBzoqm7cM2UjMD5OhXeEyKBikeCEgR1YyZfaA&s=NqSMQ1XHQaQs6cv9E3MfvAUZHdFI4o_ZJbaEC7V1nzs&e=.

    点赞 评论 复制链接分享
  • weixin_39583029 weixin_39583029 4月前

    Thanks again for help.

    点赞 评论 复制链接分享
  • weixin_39954464 weixin_39954464 4月前

    By the way, for maximum portability and correctness (e.g to ensure compiler does not optimize away multiple memory accesses) the flag variables (screenResized,...) should be of type

     C
    volatile sig_atomic_t 
    

    rather than a simple int type.

    点赞 评论 复制链接分享
  • weixin_39583029 weixin_39583029 4月前

    I knew there was something missing :)

    点赞 评论 复制链接分享

相关推荐