The current code for writing the XML files that hold the persistent
version of the IMDB data has a problem, in that if several processes (on
one host) can change an IMDB table, and thus will write the XML file
from time to time, there is a risk that two of them will write to the
XML file at the same time.
Currently, an XML file is written using this strategy: Write the XML
data to a file whose name has an added suffix ".new", and then rename
the ".new" file to the proper name. Since the file with the proper name
is replaced atomically (by the rename operation), this protects the file
from being in a partially-written state if the process or host crashes.
But if two processes attempt to write the same XML file at the same
time, they might write into the same .new file at the same time.
To fix this, I've written a patch that generates the temporary file name
using mktemp(). This ensures that two process that try to write the XML
file at the same time will use different temporary file names. Of
course, one of those files (the one that is renamed last) will become
the new XML file, but that is OK.
I'd like people to review this design and patch.
Dale
Index: sipXportLib/src/xmlparser/tinyxml.cpp
===================================================================
--- sipXportLib/src/xmlparser/tinyxml.cpp (revision 15148)
+++ sipXportLib/src/xmlparser/tinyxml.cpp (working copy)
@@ -23,6 +23,8 @@
*/
#include <ctype.h>
+#include <stdlib.h>
+
#ifndef _WIN32
# include <unistd.h>
#else
@@ -994,17 +996,25 @@
{
bool result = false;
- // the filename is what we want to write, but if we crash or are restarted
- // when it is only partially written, then the resulting file will not be
- // well-formed, so we first write to a .new file and then only when that is
- // done do we change the name to the real filename
- const char* newsuffix = ".new";
- char* newfilename = new char[strlen(filename) + strlen(newsuffix) + 1];
- if (newfilename)
+ // The filename is what we want to write, but if we crash or are
+ // restarted when it is only partially written, or if another
+ // process is writing this file at the same time, then the
+ // resulting file will not be well-formed. So we first write to a
+ // uniquely-named temporary file and then only when that is done do
+ // we rename it to the real filename (thus replacing the old file).
+
+ #define SUFFIX "XXXXXX"
+ // *newfilename is long enough to hold filename with SUFFIX appended.
+ char* newfilename = new char[strlen(filename) + sizeof (SUFFIX)];
+ // Assemble the template string for mktemp.
+ strcpy(newfilename,filename);
+ strcat(newfilename,SUFFIX);
+ // Create the unique file name.
+ mktemp(newfilename);
+
+ // Check that mktemp was able to create a file.
+ if (newfilename[0])
{
- strcpy(newfilename,filename);
- strcat(newfilename,newsuffix);
-
// The old c stuff lives on...
FILE* fp = fopen( newfilename, "w" );
if ( fp )
@@ -1018,9 +1028,10 @@
rename( newfilename, filename );
result = true;
}
- delete [] newfilename;
- }
- return result;
+ }
+ delete [] newfilename;
+
+ return result;
}
_______________________________________________
sipx-dev mailing list
[email protected]
List Archive: http://list.sipfoundry.org/archive/sipx-dev
Unsubscribe: http://list.sipfoundry.org/mailman/listinfo/sipx-dev
