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

refactor: re-export crossbeam-channel #2244

Merged

Conversation

zhangsoledad
Copy link
Member

@zhangsoledad zhangsoledad commented Aug 25, 2020

Description

re-export crossbeam-channel from facade wrapper, unify version specify.
use tilde requirements specify for crossbeam-channel, prevent automate dependency updates.

crossbeam-channel 0.4.3 has UB, may lead to jemalloc deadlock.

    frame #0: 0x00007fb34acd5620 libpthread.so.0`__lll_lock_wait + 48
    frame #1: 0x00007fb34accddf3 libpthread.so.0`__pthread_mutex_lock + 227
    frame #2: 0x000056423ada57fc ckb`_rjem_je_malloc_mutex_lock_slow at mutex.h:141:2
    frame #3: 0x000056423ada57f4 ckb`_rjem_je_malloc_mutex_lock_slow at mutex.c:84
    frame #4: 0x000056423ad6e1a8 ckb`_rjem_je_arena_tcache_fill_small at mutex.h:205:4
    frame #5: 0x000056423ad6e1a0 ckb`_rjem_je_arena_tcache_fill_small at arena.c:1261
    frame #6: 0x000056423adc1494 ckb`_rjem_je_tcache_alloc_small_hard at tcache.c:93:2
    frame #7: 0x000056423ad68618 ckb`mallocx at tcache_inlines.h:60:9
    frame #8: 0x000056423ad685e0 ckb`mallocx at jemalloc.c:1709
    frame #9: 0x000056423ad685e0 ckb`mallocx at jemalloc.c:1905
    frame #10: 0x000056423ad685e0 ckb`mallocx at jemalloc.c:2005
    frame #11: 0x000056423ad68324 ckb`mallocx at jemalloc.c:2588
    frame #12: 0x000056423b16a56c ckb`alloc::raw_vec::RawVec$LT$T$C$A$GT$::reserve::h10a313223974a7d1 + 188
    frame #13: 0x000056423b1a25f9 ckb`crossbeam_channel::flavors::array::Channel$LT$T$GT$::with_capacity::had6f123b9bc217e1 + 121

crossbeam-rs/crossbeam#533
#2235

also crossbeam-channel is not optimize for oneshot case. replace it if it is convenient.

@zhangsoledad zhangsoledad requested a review from a team August 25, 2020 04:52
@zhangsoledad zhangsoledad changed the title refactor: re-export crossbeam-channel from ckb-types refactor: re-export crossbeam-channel Aug 25, 2020
@zhangsoledad zhangsoledad force-pushed the zhangsoledad/reexport-crossbeam branch 2 times, most recently from fe5fd3c to 327b76f Compare August 25, 2020 07:25
doitian
doitian previously approved these changes Aug 25, 2020
@zhangsoledad zhangsoledad force-pushed the zhangsoledad/reexport-crossbeam branch 3 times, most recently from 8ccd065 to a5823bb Compare August 27, 2020 02:46
quake
quake previously approved these changes Aug 29, 2020
doitian
doitian previously approved these changes Aug 31, 2020
@doitian
Copy link
Member

doitian commented Aug 31, 2020

@zhangsoledad conflicting: util/logger-service/src/lib.rs

@zhangsoledad zhangsoledad dismissed stale reviews from doitian and quake via 92892e7 August 31, 2020 08:34
@zhangsoledad zhangsoledad force-pushed the zhangsoledad/reexport-crossbeam branch from ac59ce2 to 92892e7 Compare August 31, 2020 08:34
@doitian doitian requested a review from quake September 1, 2020 03:27
@yangby-cryptape
Copy link
Collaborator

bors merge

@bors
Copy link
Contributor

bors bot commented Sep 1, 2020

🕐 Waiting for PR status (Github check) to be set, probably by CI. Bors will automatically try to run when all required PR statuses are set.

@yangby-cryptape
Copy link
Collaborator

Checks can't completed and bors doesn't work.
Let administrator do merge or force-push to rerun Checks after git commit --amend.

@bors
Copy link
Contributor

bors bot commented Sep 1, 2020

GitHub status checks took too long to complete, so bors is giving up. You can adjust bors configuration to have it wait longer if you like.

@zhangsoledad
Copy link
Member Author

bors r=doitian,yangby-cryptape

@bors
Copy link
Contributor

bors bot commented Sep 1, 2020

Build succeeded:

@bors bors bot merged commit 2832363 into nervosnetwork:develop Sep 1, 2020
@zhangsoledad zhangsoledad deleted the zhangsoledad/reexport-crossbeam branch September 1, 2020 09:03
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

Successfully merging this pull request may close these issues.

4 participants