Skip to content

Commit bfd3953

Browse files
committed
MB-45378: Statically link all engines
Two issues[1,2] have so far been identified with Folly when multiple instances of Folly end up within the same process - i.e. multiple executables / DSOs present in memcached process. Both issues can be fixed by changing the visibility of certain folly symbols, but our current usage model (linking into shared libraries with visibility=hidden) is not the common model for Folly and hence the concern that this isn't a stable or safe way to continue to consume Folly. As such, move as much as possible of our linking to be static - specifically any library which uses Folly should be linked statically. This patch does this for all engines, so they are part of the memcached binary. [1]: facebook/folly#1431 [2]: facebook/folly#1558 Change-Id: Ic9633db89e9e769146e7869a5122b6720ea2e1eb Reviewed-on: http://review.couchbase.org/c/kv_engine/+/147715 Reviewed-by: Dave Rigby <[email protected]> Tested-by: Build Bot <[email protected]>
1 parent 417f820 commit bfd3953

File tree

18 files changed

+31
-106
lines changed

18 files changed

+31
-106
lines changed
Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,2 @@
1-
ADD_LIBRARY(crash_engine SHARED crash_engine.cc)
2-
SET_TARGET_PROPERTIES(crash_engine PROPERTIES PREFIX "")
3-
TARGET_LINK_LIBRARIES(crash_engine engine_utilities platform)
4-
INSTALL(TARGETS crash_engine
5-
RUNTIME DESTINATION bin
6-
LIBRARY DESTINATION lib
7-
ARCHIVE DESTINATION lib)
1+
add_library(crash_engine STATIC crash_engine.cc)
2+
target_link_libraries(crash_engine engine_utilities platform)

engines/default_engine/CMakeLists.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
ADD_LIBRARY(default_engine SHARED
1+
ADD_LIBRARY(default_engine STATIC
22
assoc.cc
33
assoc.h
44
default_engine.cc

engines/ep/CMakeLists.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -378,7 +378,7 @@ ADD_LIBRARY(ep_objs OBJECT
378378
SET_PROPERTY(TARGET ep_objs PROPERTY POSITION_INDEPENDENT_CODE 1)
379379
add_sanitizers(ep_objs)
380380

381-
ADD_LIBRARY(ep SHARED $<TARGET_OBJECTS:ep_objs>)
381+
ADD_LIBRARY(ep STATIC $<TARGET_OBJECTS:ep_objs>)
382382

383383
if (NOT WIN32)
384384
# Enable the more efficient ThreadLocalStorage model

engines/utilities/CMakeLists.txt

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,2 @@
1-
ADD_LIBRARY(engine_utilities SHARED engine_error.cc)
2-
TARGET_LINK_LIBRARIES(engine_utilities platform)
3-
4-
GENERATE_EXPORT_HEADER(engine_utilities
5-
EXPORT_MACRO_NAME ENGINE_UTILITIES_PUBLIC_API
6-
EXPORT_FILE_NAME ${PROJECT_BINARY_DIR}/include/memcached/engine_utilities_visibility.h)
7-
8-
INSTALL(TARGETS engine_utilities
9-
RUNTIME DESTINATION bin
10-
LIBRARY DESTINATION lib
11-
ARCHIVE DESTINATION lib)
1+
add_library(engine_utilities STATIC engine_error.cc)
2+
target_link_libraries(engine_utilities platform)

include/memcached/engine_error.h

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -10,18 +10,11 @@
1010
*/
1111
#pragma once
1212

13-
#include <memcached/engine_utilities_visibility.h>
1413
#include <platform/dynamic.h>
1514

1615
#include <iosfwd>
1716
#include <system_error>
1817

19-
#ifdef _MSC_VER
20-
// We need to add the right __declspec magic to the system types
21-
// to avoid msvc to spit out warnings
22-
class ENGINE_UTILITIES_PUBLIC_API std::system_error;
23-
#endif
24-
2518
namespace cb {
2619
/**
2720
* The Error enum contains all of the various error codes the engines
@@ -143,10 +136,9 @@ enum class engine_errc {
143136
*
144137
* @return The one and only instance of the error object
145138
*/
146-
ENGINE_UTILITIES_PUBLIC_API
147139
const std::error_category& engine_error_category() NOEXCEPT;
148140

149-
class ENGINE_UTILITIES_PUBLIC_API engine_error : public std::system_error {
141+
class engine_error : public std::system_error {
150142
public:
151143
engine_error(engine_errc ev, const std::string& what_arg)
152144
: system_error(int(ev), engine_error_category(), what_arg) {}
@@ -159,14 +151,11 @@ static inline std::error_condition make_error_condition(engine_errc e) {
159151
return std::error_condition(int(e), engine_error_category());
160152
}
161153

162-
ENGINE_UTILITIES_PUBLIC_API
163154
std::string to_string(engine_errc ev);
164155

165156
// GoogleTest printing function.
166-
ENGINE_UTILITIES_PUBLIC_API
167157
void PrintTo(engine_errc ev, ::std::ostream* os);
168158
// For checkeqfn
169-
ENGINE_UTILITIES_PUBLIC_API
170159
std::ostream& operator<<(std::ostream& os, cb::engine_errc ec);
171160
} // namespace cb
172161

include/statistics/cbstat_collector.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ struct ServerApi;
2626
* Formats all stats to text and immediately calls the provided
2727
* addStatFn.
2828
*/
29-
class STATISTICS_PUBLIC_API CBStatCollector : public StatCollector {
29+
class CBStatCollector : public StatCollector {
3030
public:
3131
/**
3232
* Construct a collector which calls the provided addStatFn

include/statistics/collector.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414
#include "definitions.h"
1515

1616
#include <platform/histogram.h>
17-
#include <statistics/visibility.h>
1817

1918
#include <optional>
2019
#include <string>
@@ -143,7 +142,7 @@ class BucketStatCollector;
143142
* disk_size{bucket="bucketName", scope="0x0"} 123
144143
*
145144
*/
146-
class STATISTICS_PUBLIC_API StatCollector {
145+
class StatCollector {
147146
public:
148147
using Labels = std::unordered_map<std::string_view, std::string_view>;
149148

include/statistics/definitions.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ class CBStatsKey {
7676
* stats.def.h, but can also be created directly as a fallback until
7777
* all stats have been transitioned to stats.def.h.
7878
*/
79-
struct STATISTICS_PUBLIC_API StatDef {
79+
struct StatDef {
8080
using Labels = std::unordered_map<std::string_view, std::string_view>;
8181
/**
8282
* Construct from char* to minimize code change while migrating to
@@ -188,7 +188,7 @@ struct STATISTICS_PUBLIC_API StatDef {
188188
#define STAT(statName, ...) statName,
189189
#define CBSTAT(statName, ...) statName,
190190
#define PSTAT(statName, ...) statName,
191-
enum class STATISTICS_PUBLIC_API Key {
191+
enum class Key {
192192
#include "stats.def.h"
193193

194194
enum_max

include/statistics/labelled_collector.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@
4545
* adding additional labels to every addStat call.
4646
*
4747
*/
48-
class STATISTICS_PUBLIC_API LabelledStatCollector : public StatCollector {
48+
class LabelledStatCollector : public StatCollector {
4949
public:
5050
// Allow usage of the "helper" methods defined in the base type.
5151
// They would otherwise be shadowed
@@ -147,7 +147,7 @@ class ColStatCollector;
147147
* label) to add stats for a single bucket, regardless of stat backend
148148
* implementation.
149149
*/
150-
class STATISTICS_PUBLIC_API BucketStatCollector : public LabelledStatCollector {
150+
class BucketStatCollector : public LabelledStatCollector {
151151
public:
152152
BucketStatCollector(const StatCollector& parent, std::string_view bucket);
153153
[[nodiscard]] ScopeStatCollector forScope(std::string_view scopeName,
@@ -159,7 +159,7 @@ class STATISTICS_PUBLIC_API BucketStatCollector : public LabelledStatCollector {
159159
*
160160
* See BucketStatCollector.
161161
*/
162-
class STATISTICS_PUBLIC_API ScopeStatCollector : public LabelledStatCollector {
162+
class ScopeStatCollector : public LabelledStatCollector {
163163
public:
164164
ScopeStatCollector(const BucketStatCollector& parent,
165165
std::string_view scopeName,
@@ -176,7 +176,7 @@ class STATISTICS_PUBLIC_API ScopeStatCollector : public LabelledStatCollector {
176176
*
177177
* See BucketStatCollector.
178178
*/
179-
class STATISTICS_PUBLIC_API ColStatCollector : public LabelledStatCollector {
179+
class ColStatCollector : public LabelledStatCollector {
180180
public:
181181
ColStatCollector(const ScopeStatCollector& parent,
182182
std::string_view collectionName,

include/statistics/prometheus.h

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818

1919
#include <memcached/engine_error.h>
2020
#include <platform/socket.h>
21-
#include <statistics/visibility.h>
2221

2322
namespace prometheus {
2423
// forward declaration
@@ -45,18 +44,14 @@ using GetStatsCallback =
4544
* @throws std::bad_alloc for memory allocation failures
4645
* @throws std::runtime_errors if we failed to start the exporter service
4746
*/
48-
STATISTICS_PUBLIC_API
4947
nlohmann::json initialize(const std::pair<in_port_t, sa_family_t>& config,
5048
GetStatsCallback getStatsCB,
5149
AuthCallback authCB);
5250

53-
STATISTICS_PUBLIC_API
5451
void shutdown();
5552

56-
STATISTICS_PUBLIC_API
5753
std::pair<in_port_t, sa_family_t> getRunningConfig();
5854

59-
STATISTICS_PUBLIC_API
6055
nlohmann::json getRunningConfigAsJson();
6156

6257
/**

0 commit comments

Comments
 (0)