[Impala-ASF-CR] IMPALA-12915: Use libgtest.so when built with shared libs

2024-03-25 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21163 )

Change subject: IMPALA-12915: Use libgtest.so when built with shared libs
..


Patch Set 4: Verified+1


--
To view, visit http://gerrit.cloudera.org:8080/21163
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I27d21217db219f52b072a4e5cfa1caaace35d1a2
Gerrit-Change-Number: 21163
Gerrit-PatchSet: 4
Gerrit-Owner: Quanlong Huang 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Mon, 25 Mar 2024 15:02:44 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-12915: Use libgtest.so when built with shared libs

2024-03-25 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/21163 )

Change subject: IMPALA-12915: Use libgtest.so when built with shared libs
..

IMPALA-12915: Use libgtest.so when built with shared libs

When building Impala with shared libraries, we currently link against
the static library of GTest in both libkudu_test_util.so and
unifiedbetests. libkudu_test_util.so is also linked by unifiedbetests
dynamically. When unifiedbetests exits, both binaries try to delete the
global variables of GTest, which leads to a double-free memory issue.

To fix the double-free memory issue, instead of statically linking
libgtest.a, all binaries should dynamically link libgtest.so. So when
the process exits, only libgtest.so will delete its global variables.

Tests
 - Verified the issue resolved locally

Change-Id: I27d21217db219f52b072a4e5cfa1caaace35d1a2
Reviewed-on: http://gerrit.cloudera.org:8080/21163
Reviewed-by: Impala Public Jenkins 
Tested-by: Impala Public Jenkins 
---
M CMakeLists.txt
M bin/impala-config.sh
M cmake_modules/FindGTest.cmake
3 files changed, 19 insertions(+), 10 deletions(-)

Approvals:
  Impala Public Jenkins: Looks good to me, approved; Verified

--
To view, visit http://gerrit.cloudera.org:8080/21163
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I27d21217db219f52b072a4e5cfa1caaace35d1a2
Gerrit-Change-Number: 21163
Gerrit-PatchSet: 5
Gerrit-Owner: Quanlong Huang 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Reviewer: Zoltan Borok-Nagy 


[Impala-ASF-CR] IMPALA-12915: Use libgtest.so when built with shared libs

2024-03-25 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21163 )

Change subject: IMPALA-12915: Use libgtest.so when built with shared libs
..


Patch Set 4:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/10421/ 
DRY_RUN=false


--
To view, visit http://gerrit.cloudera.org:8080/21163
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I27d21217db219f52b072a4e5cfa1caaace35d1a2
Gerrit-Change-Number: 21163
Gerrit-PatchSet: 4
Gerrit-Owner: Quanlong Huang 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Mon, 25 Mar 2024 09:57:54 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-12915: Use libgtest.so when built with shared libs

2024-03-25 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21163 )

Change subject: IMPALA-12915: Use libgtest.so when built with shared libs
..


Patch Set 4: Code-Review+2


--
To view, visit http://gerrit.cloudera.org:8080/21163
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I27d21217db219f52b072a4e5cfa1caaace35d1a2
Gerrit-Change-Number: 21163
Gerrit-PatchSet: 4
Gerrit-Owner: Quanlong Huang 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Mon, 25 Mar 2024 09:57:53 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-12915: Use libgtest.so when built with shared libs

2024-03-25 Thread Zoltan Borok-Nagy (Code Review)
Zoltan Borok-Nagy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21163 )

Change subject: IMPALA-12915: Use libgtest.so when built with shared libs
..


Patch Set 3: Code-Review+2

Thanks for fixing this!


-- 
To view, visit http://gerrit.cloudera.org:8080/21163
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I27d21217db219f52b072a4e5cfa1caaace35d1a2
Gerrit-Change-Number: 21163
Gerrit-PatchSet: 3
Gerrit-Owner: Quanlong Huang 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Mon, 25 Mar 2024 09:49:45 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-12915: Use libgtest.so when built with shared libs

2024-03-21 Thread Quanlong Huang (Code Review)
Quanlong Huang has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21163 )

Change subject: IMPALA-12915: Use libgtest.so when built with shared libs
..


Patch Set 3:

Also verified the clang-tidy job of https://gerrit.cloudera.org/c/21146/ : 
https://jenkins.impala.io/job/clang-tidy-ub2004/2805/
Previous the job failed in that patch since it adds a new metric (so triggered 
this bug).


--
To view, visit http://gerrit.cloudera.org:8080/21163
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I27d21217db219f52b072a4e5cfa1caaace35d1a2
Gerrit-Change-Number: 21163
Gerrit-PatchSet: 3
Gerrit-Owner: Quanlong Huang 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Comment-Date: Thu, 21 Mar 2024 14:17:55 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-12915: Use libgtest.so when built with shared libs

2024-03-21 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21163 )

Change subject: IMPALA-12915: Use libgtest.so when built with shared libs
..


Patch Set 3:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/15603/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


--
To view, visit http://gerrit.cloudera.org:8080/21163
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I27d21217db219f52b072a4e5cfa1caaace35d1a2
Gerrit-Change-Number: 21163
Gerrit-PatchSet: 3
Gerrit-Owner: Quanlong Huang 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Comment-Date: Thu, 21 Mar 2024 14:04:33 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-12915: Use libgtest.so when built with shared libs

2024-03-21 Thread Quanlong Huang (Code Review)
Quanlong Huang has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21163 )

Change subject: IMPALA-12915: Use libgtest.so when built with shared libs
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/21163/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/21163/2//COMMIT_MSG@7
PS2, Line 7: IMP
> nit: drop WIP?
Done. I was waiting for the toolchain patch to be merged. Just updated the 
latest toolchain build id. Also updated the commit message.



--
To view, visit http://gerrit.cloudera.org:8080/21163
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I27d21217db219f52b072a4e5cfa1caaace35d1a2
Gerrit-Change-Number: 21163
Gerrit-PatchSet: 3
Gerrit-Owner: Quanlong Huang 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Comment-Date: Thu, 21 Mar 2024 13:43:49 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-12915: Use libgtest.so when built with shared libs

2024-03-21 Thread Quanlong Huang (Code Review)
Hello Riza Suminto, Impala Public Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/21163

to look at the new patch set (#3).

Change subject: IMPALA-12915: Use libgtest.so when built with shared libs
..

IMPALA-12915: Use libgtest.so when built with shared libs

When building Impala with shared libraries, we currently link against
the static library of GTest in both libkudu_test_util.so and
unifiedbetests. libkudu_test_util.so is also linked by unifiedbetests
dynamically. When unifiedbetests exits, both binaries try to delete the
global variables of GTest, which leads to a double-free memory issue.

To fix the double-free memory issue, instead of statically linking
libgtest.a, all binaries should dynamically link libgtest.so. So when
the process exits, only libgtest.so will delete its global variables.

Tests
 - Verified the issue resolved locally

Change-Id: I27d21217db219f52b072a4e5cfa1caaace35d1a2
---
M CMakeLists.txt
M bin/impala-config.sh
M cmake_modules/FindGTest.cmake
3 files changed, 19 insertions(+), 10 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/63/21163/3
-- 
To view, visit http://gerrit.cloudera.org:8080/21163
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I27d21217db219f52b072a4e5cfa1caaace35d1a2
Gerrit-Change-Number: 21163
Gerrit-PatchSet: 3
Gerrit-Owner: Quanlong Huang 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Riza Suminto