Boost C++ Libraries: Ticket #2324: Use of tmpnam may produce spurious test results

status changed; resolution set

Robert Ramey — Tue, 16 Sep 2008 19:25:35 GMT

status new → closed
resolution → wontfix

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

status changed; resolution deleted

Dave Abrahams — Tue, 16 Sep 2008 21:49:48 GMT

status closed → reopened
resolution wontfix

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 — Fri, 19 Sep 2008 23:12:56 GMT

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 — Wed, 03 Jun 2009 00:44:04 GMT

See #590.

status changed; resolution set

Robert Ramey — Sat, 06 Jun 2009 02:10:00 GMT

status reopened → closed
resolution → wontfix

status changed; resolution deleted

Steven Watanabe — Mon, 14 Jun 2010 18:01:07 GMT

status closed → reopened
resolution wontfix

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 — Sun, 28 Nov 2010 23:15:20 GMT

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 — Sun, 28 Nov 2010 23:58:14 GMT

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 — Mon, 29 Nov 2010 20:38:00 GMT

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

Robert Ramey

Robert Ramey — Mon, 06 Dec 2010 20:30:32 GMT

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 — Tue, 07 Dec 2010 02:20:14 GMT

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.

attachment set

Dean Michael Berris — Tue, 07 Dec 2010 02:23:08 GMT

attachment → boost-serialization-test-avoid-tmpnam-issue2324.diff

Updated patch to fix compile failures on Windows with MSVC.

Robert Ramey — Tue, 07 Dec 2010 16:56:32 GMT

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 — Wed, 08 Dec 2010 02:06:01 GMT

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.

attachment set

Dean Michael Berris — Wed, 08 Dec 2010 03:04:49 GMT

attachment → boost-serialization-test-avoid-tmpnam-issue2324.2.diff

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

status, type changed; resolution set

Robert Ramey — Sat, 18 Dec 2010 19:28:39 GMT

status reopened → closed
type Bugs → Feature Requests
resolution → fixed

Uploaded to trunk