----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26806/#review56919 -----------------------------------------------------------
Ship it! This is simpler than the one I was going to propose. I like it! The one drawback is that createEmptyFile() now lies within the lock for emptyFileList, and as this is a rather time-consuming function, it will hold the lock for a while. But as releasing the lock for the creation of the file created the race condition in the first place, I can't see any alternative at this point. - Kim van der Riet On Oct. 16, 2014, 11:49 a.m., Pavel Moravec wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/26806/ > ----------------------------------------------------------- > > (Updated Oct. 16, 2014, 11:49 a.m.) > > > Review request for qpid, Gordon Sim and Kim van der Riet. > > > Bugs: QPID-6157 > https://issues.apache.org/jira/browse/QPID-6157 > > > Repository: qpid > > > Description > ------- > > For reason of the segfault, read description of relevant JIRA since > "Additional info:". > > Simply, popEmptyFile method needs to do _atomic_ test-and-create-and-grab for > an efp file, otherwise the scenario to segfault can happen. Therefore whole > method is newly under the emptyFileListMutex lock (what has some side-effect > to createEmptyFile method (called from popEmptyFile) that would be waiting on > the same already acquired lock). > > > Diffs > ----- > > trunk/qpid/cpp/src/qpid/linearstore/journal/EmptyFilePool.cpp 1631725 > > Diff: https://reviews.apache.org/r/26806/diff/ > > > Testing > ------- > > Reproducer does not show any segfault. > > Few scenarios like "EFP having 1 file only", "EFP having many files" tried > many times, no deadlock or segfault. > > Automated tests for linearstore passed. > > > Thanks, > > Pavel Moravec > >
