[zeromq-dev] libzmq - allocations, exceptions, alloc_assert and such

[Pieter Hintjens]

Sounds good to me. The extra complexity makes things more fragile IMO.

On Wed, Jan 28, 2015 at 8:16 PM, Thomas Rodgers <rodgert at twrodgers.com> wrote:
>> To understand correctly, we don't propagate such failures (as
>> exceptions or error returns), and even if we did, it'd be pointless...
>
>
> My thinly veiled agenda here is -
>
> We are already toast in this case anyway, can we just get rid of the extra
> complexity that comes with trying pretend we can actually reliably do
> something under a memory exhaustion case?
>
>
>
> On Wed, Jan 28, 2015 at 1:07 PM, Pieter Hintjens <ph at imatix.com> wrote:
>>
>> To understand correctly, we don't propagate such failures (as
>> exceptions or error returns), and even if we did, it'd be pointless...
>>
>> FWIW we added quite a lot handling in CZMQ to handle memory
>> exhaustion. I don't particularly like that, as it makes things complex
>> and it's unclear that there's any practical strategy for apps that hit
>> such errors.
>>
>>
>> On Tue, Jan 27, 2015 at 8:25 PM, Thomas Rodgers <rodgert at twrodgers.com>
>> wrote:
>> > I know sustrik has previously railed on this topic, but at the risk of
>> > rehashing some of that ground...
>> >
>> > My reading of libzmq's source suggests that it goes to some lengths to
>> > prevent raising C++ exceptions, which, given that the C API functions do
>> > not
>> > attempt to guard against propagating exceptions to the caller, is a Good
>> > Thing. The code in some places uses malloc/free, in others either
>> > placement
>> > new into pre-allocated memory or new (std::nothrow) to prevent
>> > allocations
>> > from throwing. This is all well and good as far as it goes, but it
>> > doesn't
>> > go that far -
>> >
>> > stream_engine_t, mechanism_t, metadata_t, ctx_t (for example) all use
>> > node-based STL containers (std::map, std::mutlimap) which perform
>> > allocations on insertion/copy.
>> >
>> > mechanism_t's ctor default instantiates two std::map<std::string,
>> > std::string>'s. Properties are then added to these dictionaries as they
>> > are
>> > consumed from the wire. Each property is stored as a key/value pair of
>> > std::string, which may throw on construction if it cannot allocate the
>> > requested space, and the insert may throw if it cannot allocate space
>> > for
>> > the map node.
>> >
>> > In stream_engine_t::mechanism_read() -
>> >
>> > It creates a local std::map<std::string, std::string> which it then
>> > merges
>> > properties exchanged from the transport mechanism (zap and zmtp) then
>> > constructs a metadata_t with a copy of that merged set. Though
>> > metadata_t is
>> > instantiated as follows -
>> >
>> > metadata = new (std::nothrow) metadata_t (properties);
>> >
>> > This can still throw. All std::nothrow guarantees is the allocation of
>> > space
>> > for metadata_t will not throw, it does not guard against metadata_t's
>> > ctor
>> > throwing (which can happen since it takes, and therefore must allocate
>> > space
>> > for, a copy of properties).
>> >
>> > This is perhaps somewhat academic since, in a low memory situation, you
>> > are
>> > likely hosed anyway (and on Linux you will likely never see an exception
>> > because of the OOM killer), but from a design standpoint the library
>> > currently has the ability to propagate errors back up the call stack to
>> > a
>> > client that can, in no way, do anything about them.
>> >
>> > Should the user facing API wrap and catch any propagated exceptions, and
>> > either return some error code or in the std::bad_alloc, do an
>> > alloc_assert
>> > and terminate?
>> >
>> > Another option that will address this case is to introduce a custom
>> > allocator which fails via alloc_assert, and update all STL type
>> > definitions
>> > to use that allocator.
>> >
>> > _______________________________________________
>> > zeromq-dev mailing list
>> > zeromq-dev at lists.zeromq.org
>> > http://lists.zeromq.org/mailman/listinfo/zeromq-dev
>> >
>> _______________________________________________
>> zeromq-dev mailing list
>> zeromq-dev at lists.zeromq.org
>> http://lists.zeromq.org/mailman/listinfo/zeromq-dev
>
>
>
> _______________________________________________
> zeromq-dev mailing list
> zeromq-dev at lists.zeromq.org
> http://lists.zeromq.org/mailman/listinfo/zeromq-dev
>