search

rust-pcap / pcap

Rust language pcap library
Apache License 2.0
598 stars 138 forks source link

Replace ported Unique with standard lib NonNull #230

Closed Wojtek242 closed 2 years ago

Wojtek242 commented 2 years ago

Closes #227

Wojtek242 commented 2 years ago

Draft for now as NonNull does not implement Send or Sync whilst Unique did. I'll need to look into what this means in practice.

Wojtek242 commented 2 years ago

I cannot find any information stating that operating on the same pcap_t * is thread-safe in which case it was incorrect to have the Send and Sync traits implemented. I will investigate further, but if others know something on this subject, insight would be welcome.

Stargateur commented 2 years ago

it will simple make code that was sending Capture on other thread before not compile any more. IMO it's was a bug, probably better to not take any risk anyway if we want to be Sync we need arc, and Send require pcap thread safe.

I believe pcap want to be thread safe https://github.com/the-tcpdump-group/libpcap/blob/3a7bb4ab43d14ba61eaf07921dd2e8f875281657/CHANGES#L437 but I wouldn't trust pcap on this.

Wojtek242 commented 2 years ago

I think it can be Send, but not Sync then... It can be Send since Capture creates the pcap_t * internally and so nobody else has access to this pointer. It cannot be Sync, because libpcap does not appear to be thread-safe as you pointed.

Stargateur commented 2 years ago

I'm not an expert of Sync & Send, but I think that if we consider pcap not thread safe, this mean we are not allowed to use pcap * in another thread than the original one.

Wojtek242 commented 2 years ago

My understanding of Send is that you the type can be created in thread A, its ownership passed to thread B (so A can no longer use it), and then safely dropped in thread B. Sync means you can pass a reference to another thread and both threads can concurrently use the object.

With this understanding I would say that structs that hold a NonNull<raw::pcap_t> can safely Send as Capture completely encapsulates the creation and destruction of pcap_t *. One exception is Capture<Offline> and Savefile when opened with a raw fd. However, those functions are marked unsafe and state that they assume sole ownership of the fd. I think I will therefore mark these types as Send.

However, they are not Sync as they cannot safely be accessed concurrently form multiple threads.