Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Linking with both -lprotobuf and -lprotobuf-lite #44

Closed
abuszta opened this issue Oct 10, 2014 · 3 comments
Closed

Linking with both -lprotobuf and -lprotobuf-lite #44

abuszta opened this issue Oct 10, 2014 · 3 comments

Comments

@abuszta
Copy link
Contributor

abuszta commented Oct 10, 2014

Hello,

I had my C++ binary linked dynamically together by accident with -lprotobuf and -lprotobuf-lite. Therefore after my commits (pull request #43) I noticed that binary is executing google::protobuf::internal::InitializeDefaultRepeatedFields twice and crashing after calling google::protobuf::internal::DestroyDefaultRepeatedFields second time (once for protobuf and once for protobuf-lite). Because this operates on one memory (global variables are overridden on second initialization) there is double free...

Is this valid scenario? Having two libraries linked in? Or should I fix this crash? This can be easily fixed by using GoogleOnceInit however I don't know if this is worth fixing...

What do you think?

diff --git a/src/google/protobuf/extension_set.cc b/src/google/protobuf/extension_set.cc
index 274554b..579ae31 100644
--- a/src/google/protobuf/extension_set.cc
+++ b/src/google/protobuf/extension_set.cc
@@ -1618,10 +1618,11 @@ PROTOBUF_DEFINE_DEFAULT_REPEATED(bool)

 #undef PROTOBUF_DEFINE_DEFAULT_REPEATED

+GOOGLE_PROTOBUF_DECLARE_ONCE(repeated_fields_init);
+
 struct StaticDefaultRepeatedFieldsInitializer {
   StaticDefaultRepeatedFieldsInitializer() {
-    InitializeDefaultRepeatedFields();
-    OnShutdown(&DestroyDefaultRepeatedFields);
+    GoogleOnceInit(&repeated_fields_init, &InitializeDefaultRepeatedFields);
   }
 } static_repeated_fields_initializer;

@@ -1644,6 +1645,7 @@ void InitializeDefaultRepeatedFields() {
       new RepeatedField<float>;
   RepeatedPrimitiveGenericTypeTraits::default_repeated_field_bool_ =
       new RepeatedField<bool>;
+  OnShutdown(&DestroyDefaultRepeatedFields);
 }

 void DestroyDefaultRepeatedFields() {
@xfxyjwf
Copy link
Contributor

xfxyjwf commented Oct 10, 2014

I don't think linking with both -lprotobuf and -lprotobuf-lite is a supported use case. -lprotobuf has a duplicate version of everything that -lprotobuf-lite has. You should only link with one of them.

@xfxyjwf xfxyjwf closed this as completed Oct 10, 2014
@abuszta
Copy link
Contributor Author

abuszta commented Oct 10, 2014

Ok, thanks. I wanted to be sure :)

@avtomaton
Copy link

I have the same problem, but my case is some more complex: I have already compiled dynamic libraries (i. e. from packages) used in my binary, and one of them was linked with '-lprotobuf', and the second with '-lprotobuf-lite'. I suppose that so trivial fix can be accepted to avoid this weird behaviour...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants