Fix operations with open files - #39
Conversation
Signed-off-by: Guillem Bonet <guillem@mysterium.network>
| } | ||
|
|
||
| // swap the unsafe pointer | ||
| oldfile := atomic.SwapPointer((*unsafe.Pointer)(unsafe.Pointer(&w.file)), unsafe.Pointer(newfile)) |
There was a problem hiding this comment.
reopen with atomic swap can be safe while writing concurrently.
There was a problem hiding this comment.
If I don't see the point of swapping the pointers if I have to reopen oldFile anyway. I'm not an expert so maybe you could explain me why it's best. I can modify as you say but I would still need to open oldfile again since I closed it in line 259.
There was a problem hiding this comment.
Ok, thanks for your contribution. I have to review the code workflow after leaving a long time.
|
If you'd like to add |
I think that the approach of moving/deleting open files should not be there anyway as it is allowed in unix but not in general, so I don't see a reason to create specific windows support. |
| log.Println("error in compress log file", err) | ||
| return | ||
| } | ||
| closeOnce.Do(func() { oldfile.Close() }) |
OK, I have misunderstood what this PR wants to do. FYI, someone have reported that this lib did not work well with |
|
I will do the test ASAP, then merge this PR |
Thank you for checking it and reviewing it that fast ;) |
You can probably close those now I guess, I think what I did solves that. (at least the one in english the other I don't understand :) ) |
|
that's cool ，thx for your contribution @Guillembonet |
2 participants
Found a bug when using it on windows: renaming fails because the file is open. I modified the code to only do renaming and removing operations with closed files to avoid these issues on windows. Also, there are some auto-fixes (just 3 space deletes and moving one defer) by the IDE that are nice IMO :)
Closes: #40
Signed-off-by: Guillem Bonet guillem@mysterium.network