Hi Matheus, On Thu, Jan 9, 2020 at 11:02 PM Matheus Tavares Bernardino <matheus.bernardino@xxxxxx> wrote: > > On Thu, Dec 19, 2019 at 5:27 PM Matheus Tavares Bernardino > <matheus.bernardino@xxxxxx> wrote: > > However, re-inspecting the code, it seemed to me that we might already > > have a thread-safe mechanism. The window disposal operations (at > > close_pack_windows() and unuse_one_window()) are only performed if > > window.inuse_cnt == 0. So as a thread which reads from the window will > > also previously increment its inuse_cnt, wouldn't the reading > > operation be already protected? > > > > Another concern would be close_pack_fd(), which can close packs even > > with in-use windows. However, as the mmap docs[1] says: "closing the > > file descriptor does not unmap the region". > > > > Finally, we also considered reprepare_packed_git() as a possible > > conflicting operation. But the function called by it to handle > > packfile opening, prepare_pack(), won't reopen already available > > packs. Therefore, IIUC, it will leave the opened windows intact. > > > > So, aren't perhaps the window readings performed outside the > > obj_read_mutex critical section already thread-safe? > > Any thoughts on this? My advice, if you don't hear from anyone in the next few days, is to update the commit message, the cover letter and the comments inside the patches with the information you researched and to resubmit a new version of the patch series. Thanks, Christian.
