Skip to content

Commit

Permalink
common,test: g_ceph_context->put() upon return
Browse files Browse the repository at this point in the history
prior to this change, global_init() could create a new CephContext
and set assign it to g_ceph_context. it's our responsibilty to release
the CephContext explicitly using cct->put() before the application
quites. but sometimes, we fail to do so. in this change, global_init()
will return a unique_ptr<CephContext>, which calls
`g_ceph_context->put()` in its dtor. this ensures that the CephContext
is always destroyed before the application quites. and hence flushes the
log before _log_exp_length is destroyed.

there are two cases where global_pre_init() is called directly.
- ceph_conf.cc: g_ceph_context->put() will be called by a unique_ptr<>
  deleter.
- rgw_main.cc: global_init() is called later on on the success code
  path, so it will be taken care of.

Fixes: http://tracker.ceph.com/issues/17762
Signed-off-by: Kefu Chai <kchai@redhat.com>
  • Loading branch information
tchaikov committed Nov 15, 2016
1 parent 9b836f4 commit f9fafaa
Show file tree
Hide file tree
Showing 122 changed files with 291 additions and 768 deletions.
6 changes: 3 additions & 3 deletions src/ceph_fuse.cc
Expand Up @@ -77,8 +77,9 @@ int main(int argc, const char **argv, const char *envp[]) {
}
env_to_vec(args);

global_init(NULL, args, CEPH_ENTITY_TYPE_CLIENT, CODE_ENVIRONMENT_DAEMON,
CINIT_FLAG_UNPRIVILEGED_DAEMON_DEFAULTS);
auto cct = global_init(NULL, args, CEPH_ENTITY_TYPE_CLIENT,
CODE_ENVIRONMENT_DAEMON,
CINIT_FLAG_UNPRIVILEGED_DAEMON_DEFAULTS);
for (std::vector<const char*>::iterator i = args.begin(); i != args.end(); ) {
if (ceph_argparse_double_dash(args, i)) {
break;
Expand Down Expand Up @@ -287,7 +288,6 @@ int main(int argc, const char **argv, const char *envp[]) {
foo += ::write(fd[1], &r, sizeof(r));
}

g_ceph_context->put();
free(newargv);

delete mc;
Expand Down
7 changes: 3 additions & 4 deletions src/ceph_mds.cc
Expand Up @@ -91,8 +91,9 @@ int main(int argc, const char **argv)
argv_to_vec(argc, argv, args);
env_to_vec(args);

global_init(NULL, args, CEPH_ENTITY_TYPE_MDS, CODE_ENVIRONMENT_DAEMON,
0, "mds_data");
auto cct = global_init(NULL, args,
CEPH_ENTITY_TYPE_MDS, CODE_ENVIRONMENT_DAEMON,
0, "mds_data");
ceph_heap_profiler_init();

std::string val, action;
Expand Down Expand Up @@ -228,8 +229,6 @@ int main(int argc, const char **argv)
delete msgr;
}

g_ceph_context->put();

// cd on exit, so that gmon.out (if any) goes into a separate directory for each node.
char s[20];
snprintf(s, sizeof(s), "gmon/%d", getpid());
Expand Down
5 changes: 3 additions & 2 deletions src/ceph_mgr.cc
Expand Up @@ -35,8 +35,9 @@ int main(int argc, const char **argv)
argv_to_vec(argc, argv, args);
env_to_vec(args);

global_init(NULL, args, CEPH_ENTITY_TYPE_MGR, CODE_ENVIRONMENT_DAEMON, 0,
"mgr_data");
auto cct = global_init(NULL, args, CEPH_ENTITY_TYPE_MGR,
CODE_ENVIRONMENT_DAEMON, 0,
"mgr_data");
// For consumption by KeyRing::from_ceph_context in MonClient
g_conf->set_val("keyring", "$mgr_data/keyring", false);

Expand Down
6 changes: 3 additions & 3 deletions src/ceph_mon.cc
Expand Up @@ -236,8 +236,9 @@ int main(int argc, const char **argv)
}
}

global_init(&def_args, args,
CEPH_ENTITY_TYPE_MON, CODE_ENVIRONMENT_DAEMON, flags, "mon_data");
auto cct = global_init(&def_args, args,
CEPH_ENTITY_TYPE_MON, CODE_ENVIRONMENT_DAEMON,
flags, "mon_data");
ceph_heap_profiler_init();

uuid_d fsid;
Expand Down Expand Up @@ -759,7 +760,6 @@ int main(int argc, const char **argv)
delete msgr;
delete client_throttler;
delete daemon_throttler;
g_ceph_context->put();

// cd on exit, so that gmon.out (if any) goes into a separate directory for each node.
char s[20];
Expand Down
6 changes: 3 additions & 3 deletions src/ceph_osd.cc
Expand Up @@ -101,8 +101,9 @@ int main(int argc, const char **argv)
// option, therefore we will pass it as a default argument to global_init().
def_args.push_back("--leveldb-log=");

global_init(&def_args, args, CEPH_ENTITY_TYPE_OSD, CODE_ENVIRONMENT_DAEMON,
0, "osd_data");
auto cct = global_init(&def_args, args, CEPH_ENTITY_TYPE_OSD,
CODE_ENVIRONMENT_DAEMON,
0, "osd_data");
ceph_heap_profiler_init();

// osd specific args
Expand Down Expand Up @@ -636,7 +637,6 @@ int main(int argc, const char **argv)

client_byte_throttler.reset();
client_msg_throttler.reset();
g_ceph_context->put();

// cd on exit, so that gmon.out (if any) goes into a separate directory for each node.
char s[20];
Expand Down
3 changes: 2 additions & 1 deletion src/ceph_syn.cc
Expand Up @@ -42,7 +42,8 @@ int main(int argc, const char **argv, char *envp[])
vector<const char*> args;
argv_to_vec(argc, argv, args);

global_init(NULL, args, CEPH_ENTITY_TYPE_CLIENT, CODE_ENVIRONMENT_UTILITY, 0);
auto cct = global_init(NULL, args, CEPH_ENTITY_TYPE_CLIENT,
CODE_ENVIRONMENT_UTILITY, 0);
common_init_finish(g_ceph_context);

parse_syn_options(args); // for SyntheticClient
Expand Down
15 changes: 10 additions & 5 deletions src/global/global_init.cc
Expand Up @@ -130,11 +130,12 @@ void global_pre_init(std::vector < const char * > *alt_def_args,
g_conf->complain_about_parse_errors(g_ceph_context);
}

void global_init(std::vector < const char * > *alt_def_args,
std::vector < const char* >& args,
uint32_t module_type, code_environment_t code_env,
int flags,
const char *data_dir_option, bool run_pre_init)
std::unique_ptr<CephContext, std::function<void(CephContext*)> >
global_init(std::vector < const char * > *alt_def_args,
std::vector < const char* >& args,
uint32_t module_type, code_environment_t code_env,
int flags,
const char *data_dir_option, bool run_pre_init)
{
// Ensure we're not calling the global init functions multiple times.
static bool first_run = true;
Expand Down Expand Up @@ -325,6 +326,10 @@ void global_init(std::vector < const char * > *alt_def_args,
output_ceph_version();

g_ceph_context->crush_location.init_on_startup();

using Deleter = std::function<void(CephContext*)>;
return std::unique_ptr<CephContext, Deleter>{g_ceph_context,
[](CephContext *p) {p->put();}};
}

void global_print_banner(void)
Expand Down
4 changes: 3 additions & 1 deletion src/global/global_init.h
Expand Up @@ -16,6 +16,7 @@
#define CEPH_COMMON_GLOBAL_INIT_H

#include <stdint.h>
#include <memory>
#include <vector>

#include "common/code_environment.h"
Expand All @@ -28,7 +29,8 @@ class CephContext;
* daemons and utility programs need to call. It takes care of a lot of
* initialization, including setting up g_ceph_context.
*/
void global_init(std::vector < const char * > *alt_def_args,
std::unique_ptr<CephContext, std::function<void(CephContext*)> >
global_init(std::vector < const char * > *alt_def_args,
std::vector < const char* >& args,
uint32_t module_type,
code_environment_t code_env,
Expand Down
5 changes: 3 additions & 2 deletions src/librados-config.cc
Expand Up @@ -42,8 +42,9 @@ int main(int argc, const char **argv)
bool opt_version = false;
bool opt_vernum = false;

global_init(NULL, args, CEPH_ENTITY_TYPE_CLIENT, CODE_ENVIRONMENT_UTILITY,
CINIT_FLAG_NO_DEFAULT_CONFIG_FILE);
auto cct = global_init(NULL, args, CEPH_ENTITY_TYPE_CLIENT,
CODE_ENVIRONMENT_UTILITY,
CINIT_FLAG_NO_DEFAULT_CONFIG_FILE);
common_init_finish(g_ceph_context);
for (std::vector<const char*>::iterator i = args.begin();
i != args.end(); ) {
Expand Down
3 changes: 2 additions & 1 deletion src/os/bluestore/bluefs_tool.cc
Expand Up @@ -25,7 +25,8 @@ int main(int argc, char **argv)
argv_to_vec(argc, (const char **)argv, args);
env_to_vec(args);

global_init(NULL, args, CEPH_ENTITY_TYPE_CLIENT, CODE_ENVIRONMENT_UTILITY, 0);
auto cct = global_init(NULL, args, CEPH_ENTITY_TYPE_CLIENT,
CODE_ENVIRONMENT_UTILITY, 0);
common_init_finish(g_ceph_context);
g_ceph_context->_conf->set_val(
"enable_experimental_unrecoverable_data_corrupting_features",
Expand Down
3 changes: 2 additions & 1 deletion src/rbd_replay/rbd-replay.cc
Expand Up @@ -60,7 +60,8 @@ int main(int argc, const char **argv) {

argv_to_vec(argc, argv, args);
env_to_vec(args);
global_init(NULL, args, CEPH_ENTITY_TYPE_CLIENT, CODE_ENVIRONMENT_UTILITY, 0);
auto cct = global_init(NULL, args, CEPH_ENTITY_TYPE_CLIENT,
CODE_ENVIRONMENT_UTILITY, 0);

std::vector<const char*>::iterator i;
string pool_name = "rbd";
Expand Down
10 changes: 5 additions & 5 deletions src/rgw/librgw.cc
Expand Up @@ -447,10 +447,10 @@ namespace rgw {
def_args.push_back("--keyring=$rgw_data/keyring");
def_args.push_back("--log-file=/var/log/radosgw/$cluster-$name.log");

global_init(&def_args, args,
CEPH_ENTITY_TYPE_CLIENT,
CODE_ENVIRONMENT_DAEMON,
CINIT_FLAG_UNPRIVILEGED_DAEMON_DEFAULTS);
cct = global_init(&def_args, args,
CEPH_ENTITY_TYPE_CLIENT,
CODE_ENVIRONMENT_DAEMON,
CINIT_FLAG_UNPRIVILEGED_DAEMON_DEFAULTS);

Mutex mutex("main");
SafeTimer init_timer(g_ceph_context, mutex);
Expand Down Expand Up @@ -558,7 +558,7 @@ namespace rgw {
rgw_perf_stop(g_ceph_context);

dout(1) << "final shutdown" << dendl;
g_ceph_context->put();
cct.reset();

ceph::crypto::shutdown();

Expand Down
3 changes: 2 additions & 1 deletion src/rgw/rgw_admin.cc
Expand Up @@ -2190,7 +2190,8 @@ int main(int argc, char **argv)
argv_to_vec(argc, (const char **)argv, args);
env_to_vec(args);

global_init(NULL, args, CEPH_ENTITY_TYPE_CLIENT, CODE_ENVIRONMENT_UTILITY, 0);
auto cct = global_init(NULL, args, CEPH_ENTITY_TYPE_CLIENT,
CODE_ENVIRONMENT_UTILITY, 0);
common_init_finish(g_ceph_context);

rgw_user user_id;
Expand Down
2 changes: 2 additions & 0 deletions src/rgw/rgw_lib.h
Expand Up @@ -3,6 +3,7 @@
#ifndef RGW_LIB_H
#define RGW_LIB_H

#include <memory>
#include <mutex>
#include "include/unordered_map.h"
#include "rgw_common.h"
Expand All @@ -29,6 +30,7 @@ namespace rgw {
rgw::LDAPHelper* ldh;
RGWREST rest; // XXX needed for RGWProcessEnv
RGWRados* store;
std::unique_ptr<CephContext,std::function<void(CephContext*)>> cct;

public:
RGWLib() : fec(nullptr), fe(nullptr), olog(nullptr), store(nullptr)
Expand Down
12 changes: 4 additions & 8 deletions src/rgw/rgw_main.cc
Expand Up @@ -175,9 +175,6 @@ static RGWRESTMgr *set_logging(RGWRESTMgr *mgr)
return mgr;
}

void intrusive_ptr_add_ref(CephContext* cct) { cct->get(); }
void intrusive_ptr_release(CephContext* cct) { cct->put(); }

RGWRealmReloader *preloader = NULL;

static void reloader_handler(int signum)
Expand Down Expand Up @@ -261,8 +258,10 @@ int main(int argc, const char **argv)
// Now that we've determined which frontend(s) to use, continue with global
// initialization. Passing false as the final argument ensures that
// global_pre_init() is not invoked twice.
global_init(&def_args, args, CEPH_ENTITY_TYPE_CLIENT, CODE_ENVIRONMENT_DAEMON,
flags, "rgw_data", false);
// claim the reference and release it after subsequent destructors have fired
auto cct = global_init(&def_args, args, CEPH_ENTITY_TYPE_CLIENT,
CODE_ENVIRONMENT_DAEMON,
flags, "rgw_data", false);

for (std::vector<const char*>::iterator i = args.begin(); i != args.end(); ++i) {
if (ceph_argparse_flag(args, i, "-h", "--help", (char*)NULL)) {
Expand All @@ -288,9 +287,6 @@ int main(int argc, const char **argv)

common_init_finish(g_ceph_context);

// claim the reference and release it after subsequent destructors have fired
boost::intrusive_ptr<CephContext> cct(g_ceph_context, false);

int r = rgw_tools_init(g_ceph_context);
if (r < 0) {
derr << "ERROR: unable to initialize rgw tools" << dendl;
Expand Down
5 changes: 3 additions & 2 deletions src/rgw/rgw_object_expirer.cc
Expand Up @@ -60,8 +60,9 @@ int main(const int argc, const char **argv)
argv_to_vec(argc, argv, args);
env_to_vec(args);

global_init(NULL, args, CEPH_ENTITY_TYPE_CLIENT, CODE_ENVIRONMENT_DAEMON,
CINIT_FLAG_UNPRIVILEGED_DAEMON_DEFAULTS, "rgw_data");
auto cct = global_init(NULL, args, CEPH_ENTITY_TYPE_CLIENT,
CODE_ENVIRONMENT_DAEMON,
CINIT_FLAG_UNPRIVILEGED_DAEMON_DEFAULTS, "rgw_data");

for (std::vector<const char *>::iterator i = args.begin(); i != args.end(); ) {
if (ceph_argparse_double_dash(args, i)) {
Expand Down
3 changes: 2 additions & 1 deletion src/rgw/rgw_token.cc
Expand Up @@ -64,7 +64,8 @@ int main(int argc, char **argv)
argv_to_vec(argc, (const char **)argv, args);
env_to_vec(args);

global_init(NULL, args, CEPH_ENTITY_TYPE_CLIENT, CODE_ENVIRONMENT_UTILITY, 0);
auto cct = global_init(NULL, args, CEPH_ENTITY_TYPE_CLIENT,
CODE_ENVIRONMENT_UTILITY, 0);
common_init_finish(g_ceph_context);

char *v{nullptr};
Expand Down
2 changes: 1 addition & 1 deletion src/test/ObjectMap/test_keyvaluedb_iterators.cc
Expand Up @@ -1786,7 +1786,7 @@ int main(int argc, char *argv[])
vector<const char*> args;
argv_to_vec(argc, (const char **) argv, args);

global_init(NULL, args, CEPH_ENTITY_TYPE_CLIENT, CODE_ENVIRONMENT_UTILITY, 0);
auto cct = global_init(NULL, args, CEPH_ENTITY_TYPE_CLIENT, CODE_ENVIRONMENT_UTILITY, 0);
common_init_finish(g_ceph_context);
::testing::InitGoogleTest(&argc, argv);

Expand Down
3 changes: 2 additions & 1 deletion src/test/ObjectMap/test_object_map.cc
Expand Up @@ -551,7 +551,8 @@ int main(int argc, char **argv) {
vector<const char*> args;
argv_to_vec(argc, (const char **)argv, args);

global_init(NULL, args, CEPH_ENTITY_TYPE_CLIENT, CODE_ENVIRONMENT_UTILITY, 0);
auto cct = global_init(NULL, args, CEPH_ENTITY_TYPE_CLIENT,
CODE_ENVIRONMENT_UTILITY, 0);
common_init_finish(g_ceph_context);
::testing::InitGoogleTest(&argc, argv);
return RUN_ALL_TESTS();
Expand Down
3 changes: 2 additions & 1 deletion src/test/TestSignalHandlers.cc
Expand Up @@ -71,7 +71,8 @@ int main(int argc, const char **argv)
argv_to_vec(argc, argv, args);
env_to_vec(args);

global_init(NULL, args, CEPH_ENTITY_TYPE_CLIENT, CODE_ENVIRONMENT_UTILITY, 0);
auto cct = global_init(NULL, args, CEPH_ENTITY_TYPE_CLIENT,
CODE_ENVIRONMENT_UTILITY, 0);
common_init_finish(g_ceph_context);

test_fn_t fn = NULL;
Expand Down
3 changes: 2 additions & 1 deletion src/test/TestTimers.cc
Expand Up @@ -255,7 +255,8 @@ int main(int argc, const char **argv)
argv_to_vec(argc, argv, args);
env_to_vec(args);

global_init(NULL, args, CEPH_ENTITY_TYPE_CLIENT, CODE_ENVIRONMENT_UTILITY, 0);
auto cct = global_init(NULL, args, CEPH_ENTITY_TYPE_CLIENT,
CODE_ENVIRONMENT_UTILITY, 0);
common_init_finish(g_ceph_context);

int ret;
Expand Down
16 changes: 1 addition & 15 deletions src/test/admin_socket.cc
Expand Up @@ -12,15 +12,12 @@
*
*/

#include <gtest/gtest.h>

#include "common/Mutex.h"
#include "common/Cond.h"
#include "common/admin_socket.h"
#include "common/admin_socket_client.h"
#include "common/ceph_argparse.h"
#include "global/global_init.h"
#include "global/global_context.h"
#include "test/unit.h"

#include <stdint.h>
#include <string.h>
Expand Down Expand Up @@ -293,17 +290,6 @@ TEST(AdminSocket, bind_and_listen) {
}
}

int main(int argc, char **argv) {
vector<const char*> args;
argv_to_vec(argc, (const char **)argv, args);

vector<const char*> def_args;
global_init(&def_args, args, CEPH_ENTITY_TYPE_CLIENT, CODE_ENVIRONMENT_UTILITY, 0);
common_init_finish(g_ceph_context);
::testing::InitGoogleTest(&argc, argv);
return RUN_ALL_TESTS();
}

/*
* Local Variables:
* compile-command: "cd .. ;
Expand Down
2 changes: 1 addition & 1 deletion src/test/bench/small_io_bench_dumb.cc
Expand Up @@ -95,7 +95,7 @@ int main(int argc, char **argv)
ceph_options.push_back(i->c_str());
}

global_init(
auto cct = global_init(
&def_args, ceph_options, CEPH_ENTITY_TYPE_CLIENT,
CODE_ENVIRONMENT_UTILITY,
CINIT_FLAG_NO_DEFAULT_CONFIG_FILE);
Expand Down
2 changes: 1 addition & 1 deletion src/test/bench/small_io_bench_fs.cc
Expand Up @@ -106,7 +106,7 @@ int main(int argc, char **argv)
ceph_options.push_back(i->c_str());
}

global_init(
auto cct = global_init(
&def_args, ceph_options, CEPH_ENTITY_TYPE_CLIENT,
CODE_ENVIRONMENT_UTILITY,
CINIT_FLAG_NO_DEFAULT_CONFIG_FILE);
Expand Down
2 changes: 1 addition & 1 deletion src/test/bench/tp_bench.cc
Expand Up @@ -142,7 +142,7 @@ int main(int argc, char **argv)
ceph_options.push_back(i->c_str());
}

global_init(
auto cct = global_init(
&def_args, ceph_options, CEPH_ENTITY_TYPE_CLIENT,
CODE_ENVIRONMENT_UTILITY,
CINIT_FLAG_NO_DEFAULT_CONFIG_FILE);
Expand Down
3 changes: 2 additions & 1 deletion src/test/bench_log.cc
Expand Up @@ -39,7 +39,8 @@ int main(int argc, const char **argv)
argv_to_vec(argc, argv, args);
env_to_vec(args);

global_init(NULL, args, CEPH_ENTITY_TYPE_OSD, CODE_ENVIRONMENT_UTILITY, 0);
auto cct = global_init(NULL, args, CEPH_ENTITY_TYPE_OSD,
CODE_ENVIRONMENT_UTILITY, 0);

utime_t start = ceph_clock_now(NULL);

Expand Down

0 comments on commit f9fafaa

Please sign in to comment.