Use of tmpnam may produce spurious test results

[Dave Abrahams]

As described in ​http://www.dwheeler.com/secure-programs/Secure-Programs-HOWTO/avoid-race.html, tmpnam is subject to race conditions, which makes it especially bad for use in testing when testers are exploiting parallelism via threads or processes. Several files in the library are using tmpnam when they should use mkstemp.

[Robert Ramey]

This is used in the serialization test programs - none of which exploit parallelism via threads nor processes. I don't remember now, but I found mkstemp unsuitable for the purpose at hand.

Robert Ramey

[Dave Abrahams]

Yes, I know it's used for testing. Whether or not tests are run in parallel is not under your control as the author of the library and its tests. A tester may run bjam with -jN and several tests will run simultaneously, and that may be a feature of the testing system or simply how the tester configures the test machine. The ability to exploit all the resources of the testing machines is extremely important to reducing our test turnaround time.

[Robert Ramey]

when I looked into using mktemp I couldn't figure out how to use it to open temporary C++ stream as on fstream(?).

mkstemp returns a file handle, and I couldn't figure out how to initialize a stream with the handle to an already open file. I saw no portable way to avoid using tmpname.

Robert Ramey

[Steven Watanabe]

See #590.

[Steven Watanabe]

Boost.Filesystem v3 contains a unique_path function which should behave better than tmpnam, since it uses CryptGenRandom and /dev/urandom to generate the file name. Once unique_path is stable, can you switch to it?

[Dean Michael Berris]

Actually, you can use mkstemp to create the file while providing a pattern for the temporary file, and be sure that the file is there when the call returns. This means you don't need to use the file descriptor it returns and just need to use the generated filename of the temporary file afterwards.

You can even close the file descriptor returned by mkstemp immediately to avoid leaking open file descriptors.

I'll try to come up with the patch to illustrate what I mean by this.

[anonymous]

Scratch that, it looks like it's easier to just use Boost.Filesystem v3's unique_path. I'll work on a patch to get that done.

[Robert Ramey]

This looks pretty good, I'll try it next time I stick my fingers in here.

Robert Ramey

[Robert Ramey]

Did you actually test this yourself?

When I compile with my vc 7.1 system I get

Compiling... test_array.cpp Linking to lib file: boost_filesystem-vc71-mt-gd-1_45.lib Linking to lib file: boost_system-vc71-mt-gd-1_45.lib c:\BoostRelease\libs\serialization\test\test_tools.hpp(96) : error C2039: 'str' : is not a member of 'boost::filesystem3::path'

c:\BoostRelease\libs\serialization\vc7ide\..\..\..\boost\filesystem\v3\path.hpp(54) : see declaration of 'boost::filesystem3::path'

c:\BoostRelease\libs\serialization\test\test_tools.hpp(96) : error C2228: left of '.c_str' must have class/struct/union type Linking to lib file: boost_serialization-vc71-mt-gd-1_45.lib Linking to lib file: boost_serialization-vc71-mt-gd-1_45.lib

Robert Ramey

[Dean Michael Berris]

Replying to ramey:

Did you actually test this yourself?

Yes, but only on Linux. I'll fix the patch for windows to address the issue, please expect an update in a few minutes.

[Dean Michael Berris]

Updated patch to fix compile failures on Windows with MSVC.

[Robert Ramey]

I ran this with bjam and things seemed to work fine.

I do have a few questions however.

a) I didn't find any documentation in the boost/filesystem regarding unique_path. This concerned me somewhat but not all that much.

b) I couldn't figure out in which directory the temporary files are created. For the windows platform, there is special code to be sure that they are created in the $TMP or $TEMP directory. But, for *nix, I relied upon the fact that tmpnam creates it's files in these temporary directories. It seems that this fix looses that that. So does this guarantee that these files are created in the $TMP directory and if not, can it be modified to guarantee this?

Robert Ramey

[Dean Michael Berris]

Replying to ramey:

I ran this with bjam and things seemed to work fine.

Cool.

I do have a few questions however.

a) I didn't find any documentation in the boost/filesystem regarding unique_path. This concerned me somewhat but not all that much.

http://www.boost.org/doc/libs/1_45_0/libs/filesystem/v3/doc/reference.html#unique_path

This is in Boost.Filesystem V3.

b) I couldn't figure out in which directory the temporary files are created. For the windows platform, there is special code to be sure that they are created in the $TMP or $TEMP directory. But, for *nix, I relied upon the fact that tmpnam creates it's files in these temporary directories. It seems that this fix looses that that. So does this guarantee that these files are created in the $TMP directory and if not, can it be modified to guarantee this?

On Unix, the path in the patch would just create the file where the binary was being run. I was thinking since this was really a temporary file, that the tests would actually delete the file once they're done.

The answer would be yes, that the pattern can be modified to create a path which actually uses the $TMP environment variable as part of the pattern, or have it hard-coded to be '/tmp/serialization-%%%%%%%%%%%%%%%%'.

I'll check to see whether I can find a portable UNIX way of at least getting the correct temporary directory path.

[Dean Michael Berris]

Updated patch to use boost::archive::tmpdir to get the correct directory for temporary storage on Unix.

[Robert Ramey]

Uploaded to trunk