Skip to content

(timer) Clarify lifetime requirements on Envoy::Event::Timer#7812

Merged
mattklein123 merged 1 commit intoenvoyproxy:masterfrom
aunu53:patch-1
Aug 7, 2019
Merged

(timer) Clarify lifetime requirements on Envoy::Event::Timer#7812
mattklein123 merged 1 commit intoenvoyproxy:masterfrom
aunu53:patch-1

Conversation

@aunu53
Copy link
Copy Markdown
Contributor

@aunu53 aunu53 commented Aug 2, 2019

I attempted to use a Timer from outside of Envoy in my code, and ran into destructor problems. The Timer was being destroyed after the Envoy server was already destroyed, causing a segfault (partial stack trace below). Is this a bug in the code or a bug in the comments?

Risk Level: N/A (Comment only)

(gdb) bt
#0  0x00007ffff7eb9970 in pthread_mutex_lock () from /usr/grte/v4/lib64/libpthread.so.0
#1  0x0000555563d481c0 in evthread_posix_lock (mode=0, lock_=0xcdcdcdcdcdcdcdcd) at libevent/src/evthread_pthread.c:75
#2  0x0000555563d5f356 in event_del_ (ev=0x7fffbd812428, blocking=2) at libevent/src/event.c:2811
#3  0x0000555563d5db9a in event_del (ev=0x7fffbd812428) at libevent/src/event.c:2821
#4  0x0000555563d47ea5 in Envoy::Event::ImplBase::~ImplBase (this=0x7fffbd812428) at envoy/src/source/common/event/event_impl_base.cc:10
#5  0x0000555563d47e17 in Envoy::Event::TimerImpl::~TimerImpl (this=0x7fffbd812420) at envoy/src/source/common/event/timer_impl.h:24
#6  0x0000555563d47e4c in Envoy::Event::TimerImpl::~TimerImpl (this=0x7fffbd812420) at envoy/src/source/common/event/timer_impl.h:24
#7  0x000055555de3a2df in std::__u::default_delete<Envoy::Event::Timer>::operator() (this=0x7fffbdfb6718, __ptr=0x7fffbd812420)
    at crosstool/v18/stable/toolchain/bin/../include/c++/v1/memory:2344
#8  0x000055555de3a043 in std::__u::unique_ptr<Envoy::Event::Timer, std::__u::default_delete<Envoy::Event::Timer> >::~unique_ptr (this=0x7fffbdfb6718)
    at crosstool/v18/stable/toolchain/bin/../include/c++/v1/memory:2560

@aunu53 aunu53 changed the title (timer) Clarify lifetime requirements on Envoy::Event::Timer [WIP but please look at this] (timer) Clarify lifetime requirements on Envoy::Event::Timer Aug 2, 2019
@mattklein123
Copy link
Copy Markdown
Member

mattklein123 commented Aug 2, 2019

Yes this is correct. Please fix format and we can merge.

@mattklein123 mattklein123 self-assigned this Aug 3, 2019
@aunu53
Clarify lifetime requirements on Envoy::Event::Timer
0f53248 
Signed-off-by: Auni Ahsan <auni@google.com>

Signed-off-by: Auni Ahsan <auni@google.com>
@aunu53 aunu53 changed the title [WIP but please look at this] (timer) Clarify lifetime requirements on Envoy::Event::Timer \(timer) Clarify lifetime requirements on Envoy::Event::Timer Aug 7, 2019
@aunu53 aunu53 changed the title \(timer) Clarify lifetime requirements on Envoy::Event::Timer (timer) Clarify lifetime requirements on Envoy::Event::Timer Aug 7, 2019
mattklein123
mattklein123 approved these changes Aug 7, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@mattklein123 mattklein123 merged commit 3d4f6fa into envoyproxy:master Aug 7, 2019
@aunu53 aunu53 deleted the patch-1 branch August 7, 2019 17:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mattklein123 mattklein123 mattklein123 approved these changes

Assignees

@mattklein123 mattklein123

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@aunu53 @mattklein123