Details
-
Bug
-
Resolution: Won't Fix
-
Major - P3
-
None
-
None
-
None
-
None
Description
Although the documentation for mongoc_gridfs_create_file_from_stream() does not clearly state whether the user is responsible for cleaning up the mongoc_stream_t on success/failure, from reading the source code, examples, and tests in the repo, it seems like the intended contract is:
- on success, the mongoc_stream_t will be cleaned up by the callee
- on failure, the mongoc_stream_t should be cleaned up by the caller
However, callers cannot simply call mongoc_stream_destroy() if the function returns NULL, because the stream is only valid on certain error paths, not all error paths.
Looking at https://github.com/mongodb/mongo-c-driver/blob/1.17.1/src/libmongoc/src/mongoc/mongoc-stream.c#L78:
for (;;) {
|
r = mongoc_stream_read (
|
stream, iov.iov_base, MONGOC_GRIDFS_STREAM_CHUNK, 0, timeout);
|
|
|
if (r > 0) {
|
iov.iov_len = r;
|
if (mongoc_gridfs_file_writev (file, &iov, 1, timeout) < 0) {
|
MONGOC_ERROR ("%s", file->error.message);
|
mongoc_gridfs_file_destroy (file);
|
RETURN (NULL); // #1
|
}
|
} else if (r == 0) {
|
break;
|
} else {
|
MONGOC_ERROR ("Error reading from GridFS file source stream");
|
mongoc_gridfs_file_destroy (file);
|
RETURN (NULL); // #2
|
}
|
}
|
|
|
mongoc_stream_failed (stream); // <--- stream destroyed here
|
|
|
if (-1 == mongoc_gridfs_file_seek (file, 0, SEEK_SET)) {
|
MONGOC_ERROR ("%s", file->error.message);
|
mongoc_gridfs_file_destroy (file);
|
RETURN (NULL); // #3
|
}
|
The three NULL return paths have been labeled. For #1 and #2, the mongoc_stream_t will be valid after the call returns, but for #3 it will have already been deallocated by mongoc_stream_failed().
Given there is no way to programmatically distinguish the error return paths, the only safe behavior currently is to avoid calling mongoc_stream_destroy() on error, which leads to resource leaks.