View Issue Details

IDProjectCategoryView StatusLast Update
0005939unrealircdpublic2021-06-30 13:10
Reportersyzop Assigned Tosyzop  
PrioritynormalSeverityminorReproducibilityhave not tried
Status resolvedResolutionfixed 
Product Version5.2.0.1 
Fixed in Version5.2.1-rc1 
Summary0005939: Windows logging max file size reached x1000
DescriptionOn a bigger log file, with presumably some limit, on Windows you see:
Some line logged
Max file size reached, starting new log file
Another line logged
Max file size reached, starting new log file
Third line logged
Max file size reached, starting new log file
etc... etc....

I guess the rename operation didn't work? we also don't catch any errors in the rename() at all so that isn't good either.
(although.. OTOH.. it is a bit hard to log those errors, but we could at least send them to ircops i guess)
TagsNo tags attached.
3rd party modules

Activities

syzop

2021-06-30 13:10

administrator   ~0022063

Fixed in https://github.com/unrealircd/unrealircd/commit/329f48334c10271caebe23faa3b0d856812543bc and then revised in https://github.com/unrealircd/unrealircd/commit/696d5f05fb7202c9c8067701b247bfc05d5838c3

commit 329f48334c10271caebe23faa3b0d856812543bc
Author: Bram Matthys <syzop@vulnscan.org>
Date: Wed Jun 30 10:55:26 2021 +0200

    I/O engine: track if a fd is a file or socket, needed for Windows.
    
    This fixes a file descriptor leak in Windows that happened in the
    logging code. The most visible effect of this was if you had a
    log::maxsize set then on Windows you would see:
    "Max file size reached, starting new log file"
    Every other line, forever (and not actually starting a new log).
    
    fd_close() previously did not close the file descriptor of a file
    on Windows because on Windows it needs to call close() for a file
    and closesocket() for a socket, and it always did the latter.
    On *NIX it's more easy and you can just always close() any fd.

..and...


commit 696d5f05fb7202c9c8067701b247bfc05d5838c3 (HEAD -> unreal52, origin/unreal52, origin/HEAD)
Author: Bram Matthys <syzop@vulnscan.org>
Date: Wed Jun 30 11:23:07 2021 +0200

    Last argument in fd_open() is now used to indicate what should be done on a
    later fd_close() call. This also removes fd_map() since fd_open w/FDCLOSE_NONE
    now does that.
    
    * If you use fd_socket() or fd_accept(), then no change.
      When fd_close() is called we call close() on *NIX and closesocket() on Win.
    * If you use fd_fileopen(), then no change.
      When fd_close() is called we will call close() on both *NIX and Win.
    * If you used fd_open() and then fd_unmap() because you didn't want us
      to close the socket, then use fd_open() with FDCLOSE_NONE and
      just call fd_close() instead of fd_unmap().
      We will not actually close the fd in fd_close() (FDCLOSE_NONE).
    * If you called fd_open() with other intentions then either specify a
      FDCLOSE_SOCKET / FDCLOSE_FILE as the last argument, or more likely:
      don't use fd_open() at all and use fd_socket() or fd_fileopen() instead.
    
    For reasons on this change, see previous patch. This way is more sane and
    makes it harder to make mistakes even beyond Windows-specific issues.

Issue History

Date Modified Username Field Change
2021-06-28 19:45 syzop New Issue
2021-06-28 19:45 syzop Product Version => 5.2.0.1
2021-06-28 19:45 syzop Assigned To => syzop
2021-06-28 19:45 syzop Status new => confirmed
2021-06-30 13:09 syzop View Status private => public
2021-06-30 13:10 syzop Status confirmed => resolved
2021-06-30 13:10 syzop Resolution open => fixed
2021-06-30 13:10 syzop Fixed in Version => 5.2.1-rc1
2021-06-30 13:10 syzop Note Added: 0022063