23_allocator test case can consume all system memory

• Key: STDCXX-440
• Type: Bug
• Status: Resolved
• Resolution: Fixed
• Priority: Critical
• Assignee: Andrew Black
• Reporter: Eric Lemings
• Component/s: Tests
• Affects Version/s: 4.2.0
• Fix Version/s: 4.2.0

Description

The 22_allocator test case has been observed to consume all primary and virtual memory bringing the unfortunate system to its knees.

Comments

[Eric Lemings]

This is the output of the 'top' command from the most recent instance where this behavior was observed.

Assigned to Andrew.

[Andrew Black]

A resolution for this issue has been committed in the referenced subversion change. However, I do not believe this will be the final solution.

This solution uses the ulimt function of the exec utility to limit tests to allocating 1 GB of memory. In addition, I chose to limit examples to allocating 256 MB of memory.

A similar limit should be applied for the locale tests, but I am uncertain as to what a suitable number would be.

One concern I have is that users are unable to override this value, as the --ulimit switch operates on a last takes precedence mode of operation. The resolution to this concern would be to prepend the switch to the runflags variable.

A second concern I have is that some previous successful tests may now segfault due to being unable to allocate the memory they require. Manual testing on my local box indicates that the 22_codecvt_iso2022 test (not yet in subversion) exhibits this behavior.

--Andrew Black

[Martin Sebor]

I spent some time looking at this problem a bit and it seems to me that the loop in the limit_process() function in exec.cpp that's (presumably) supposed to iterate over all the limits hardcoded in the body of the function, skipping those that weren't set on the command line, instead appears to break after finding the first one that's not set. Since the first one is RLIMIT_CORE and the only one we're setting is last (RLIMIT_AS), the function never does set any limit. That seems really strange because the function hasn't changed since it was implemented in http://svn.apache.org/viewvc?view=rev&revision=452253 and I don't see how it could have ever worked.

Incidentally, parse_limit_opts() in cmdopt.cpp hardcodes the same set of limits as limit_process(), which means that if we update one of them (e.g., add a new limit) we'll need to remember to update the other, which is all too easy to forget to do. The data in both functions should be factored out into a common data structure shared by the function to eliminate this potential problem.

[Andrew Black]

I'm not certain I agree with your conclusion about a misbehavior in the limit_process logic Martin.

The flow control operator used in limit_process is a 'continue', not a 'break'. Unless I'm seriously mistaken, this essentially skips over the remainder of the for loop, sending the flow back to the start of the loop and incrementing the counter.

[Andrew Black]

Resolving issue as Fixed. The latest subversion change fixes the misbehavior observed. This change has been verified under manual testing. I think the reason this bug wasn't spotted before was because the only limit being set with any regularity was RLIMIT_CORE, which is the first limit which was being checked in the loop.

Refactoring the common data shared between parse_limit_opts() and limit_process() is outside the scope of this issue and should probably be filed as an enhancement for 4.2.1.

If it is necessary to reevaluate the limits being set, this issue can be reopened.

[Martin Sebor]

We need to put together tests for the most important functionality in exec, especially something as hard to detect as the limits working correctly.

[Andrew Black]

Adding tests for the exec utility probably should be scheduled for 4.2.1, in a separate JIRA issue.

One piece of fallout I'm observing is that examples on hpux-11.23-ia64 are failing to run with messages like
./run (accumulate): execv ("./accumulate", ...) error: Not enough space

My reading of this error message, and the fact that it surfaced only after r579307 was checked into subversion, is that 256 MB isn't a high enough as limit for executables on this platform. As such, the solution likely is to raise this limit, probably to 384 MB, though testing would be required. The only other solution I see would be to reduce the memory usage of the library, which likely isn't possible.

[Martin Sebor]

I think the error you're pointing out is ENOSP (Not enough space on devivce), and is caused by a faulty disk on the server where the builds ran (talus). The error I would expect to see as a result of the changes to exec is SIGSEGV.